Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vite-node --watch doesn't close process before restarting #2334

Closed
6 tasks done
KyleAMathews opened this issue Nov 15, 2022 · 21 comments
Closed
6 tasks done

vite-node --watch doesn't close process before restarting #2334

KyleAMathews opened this issue Nov 15, 2022 · 21 comments
Labels
documentation Improvements or additions to documentation

Comments

@KyleAMathews
Copy link
Contributor

Describe the bug

I created a simple Typescript Express app that I wanted to try vite-node with. It runs fine but when I edit the src code, I get this error:

[vite-node] Failed to execute file:                       
Error: listen EADDRINUSE: address already in use :::4000

Which shows that vite-node isn't closing the previous process when in watch mode.

Reproduction

https://stackblitz.com/edit/vitest-dev-vitest-7yrhdv?file=package.json,server.ts&initialPath=__vitest__

changing what is logged out starts to accumulate multiple log entries.

System Info

n/a

Used Package Manager

npm

Validations

@sheremet-va
Copy link
Member

sheremet-va commented Nov 15, 2022

Not sure if it's vite-node responsibility? You can use import.meta.hot to do this:

if (import.meta.hot) {
  import.meta.hot.accept(() => {
    server.close()
  })
}

@KyleAMathews
Copy link
Contributor Author

HMR doesn't work for node processes — vite-node is creating new node process on changes and leaving the old ones running. I just expect the old one to be killed before the new one starts.

@sheremet-va
Copy link
Member

import.meta.hmr is the way for developer to control watch mode behaviour

@KyleAMathews
Copy link
Contributor Author

KyleAMathews commented Nov 15, 2022

I'm using vite-node directly in an npm script. Killing the old node process there is standard behavior for --watch e.g. node's new --watch or nodemon.

@sheremet-va
Copy link
Member

I'm using vite-node directly in an npm script. Killing the old node process there is standard behavior for --watch e.g. node's new --watch or nodemon.

vite-node doesn't start new process on file change, so there is no such thing as "old node process", it reevaluates module graph on file change. To react to changes in watch mode you need to hook up into its lifecycle.

@sheremet-va
Copy link
Member

I'm also not sure what do you expect to happen in a reproduction?

image

This looks expected to me

@david-plugge
Copy link

david-plugge commented Nov 19, 2022

I ran into the same issue with a fastify server.

By storing the server in globalThis and if present stop it before creating a new one i was able to make it work.

declare global {
	// eslint-disable-next-line no-var
	var __app: FastifyInstance;
}

if (globalThis.__app) {
	await globalThis.__app.close();
}

const app = (globalThis.__app = fastify({
	logger: true
}));

Edit:

Nevermind! You suggestion works like a charm @sheremet-va

if (import.meta.hot) {
	import.meta.hot.accept(async () => {
		await app.close();
	});
}

@KyleAMathews
Copy link
Contributor Author

Can someone tell me what I'm doing wrong here then?

const http = require('http');

const requestListener = function (req, res) {
  res.writeHead(200);
  res.end('Hello, World!');
}

const server = http.createServer(requestListener);
server.listen(8080, () => {
  console.log(`listening2`)
});

if (import.meta.hot) {
  import.meta.hot.accept(() => {
    server.close()
  })
}

I started the server with npx vite-node --watch index.ts and it outputted listening. I then changed that to output listening2 and there was the immediate "address already in use" error.

Screen Shot 2022-11-19 at 2 00 40 PM

@david-plugge
Copy link

Alright, i have to revert what i´ve said. The error still appears, i just didn´t fully tested it.

Using vite-node for running servers in development doesnt seems to work at the moment.
What could eventually solve it would be an option to completely get rid of the current node process that runs the code and spawn a new process with the latest code (basically rebuild the whole app and run it like nodemon). I´ve tried disabling hmr but that just disables all reload capabilities.

I´d love to use vite for this due to the speed and plugin ecosystem but i wonder if this kind of usage is in the scope of vite-node or if we should rather use something else to run our server (something like nodemon/ts-node-dev).

@sheremet-va
Copy link
Member

So, Vite considers editing the main entry here as a full reload type of HMR, so it won't call dispose, and it calls accept too late. But I found a way to close server before ViteNode reevaluates file:

if (import.meta.hot) {
  import.meta.hot.on("vite:beforeFullReload", () => {
    server.close();
  });
}

Maybe it's a good idea to also add dispose just in case:

import.meta.hot.dispose(() => {
  server.close()
});

It will be called only if HMR is triggered not as a full reload. According to Vite docs it's meant to clear all side effects.

We should probably add this information to docs.

@GuskiS
Copy link

GuskiS commented Jan 20, 2023

@sheremet-va Seems like it is working, however if I do something like this:

if (import.meta.hot) {
  import.meta.hot.on("vite:beforeFullReload", () => {
    console.log("Reload");
  });
}

it keeps adding listeners and it prints Reload multiple times on single file save. Any ideas?

@jgoux
Copy link
Contributor

jgoux commented Feb 2, 2023

@GuskiS I had the same issue! #2792 should fix it. 😄

@GuskiS
Copy link

GuskiS commented Feb 3, 2023

@jgoux That seems to have fixed the issue, but it seems there is another one - if you quickly double save file, I can still get port in use error which crashes server. It seems that if callback is async, vite-node doesn't wait for it to finish 😞

@ducan-ne
Copy link

ducan-ne commented Apr 17, 2023

@jgoux That seems to have fixed the issue, but it seems there is another one - if you quickly double save file, I can still get port in use error which crashes server. It seems that if callback is async, vite-node doesn't wait for it to finish 😞

I faced this issue today, don't think there is someone faced it already 😄

The temporary solution could be ... setTimeout

@pthieu
Copy link

pthieu commented May 29, 2023

Above solution doesn't work for me, can someone let me know what I'm doing wrong?

/src/index.ts

import config from './config';
import ApiRouter from '~/api';
import app from '~/app';

const PORT = config.PORT;

export const server = app(ApiRouter);

const _server = server.listen(PORT, () => {
  console.log(`Server listening port ${PORT}`);
});

if (import.meta.hot) {
  console.log('hot reload')
  async function killServer() {
    await _server.close((err) => {
      console.log('server closed')
      process.exit(err ? 1 : 0)
    });
  }

  import.meta.hot.on("vite:beforeFullReload", () => {
    console.log("full reload")
    killServer()
  });

  import.meta.hot.dispose(() => {
    console.log('dispose')
    killServer()
  });
}

And my run command is vite-node --watch src/

output:

pn dev
> vite-node --watch src/

hot reload
Server listening port 9000
      <<<<< made some changes here
hot reload
[vite-node] Failed to execute file:
 Error: listen EADDRINUSE: address already in use :::9000

Looks like neither of the callbacks are getting hit, so the server doesn't get killed.

EDIT: looks like run command is incorrect, changing it to this works:

vite-node --watch src/*.ts

I have another issue now, where the server reloads, but the files don't actually get updated. Anyone hit this before?

EDIT 2: ok so some deeper files are getting updated but it takes a long time (maybe ~10s), anyone face this before?

@scarf005
Copy link
Contributor

scarf005 commented Jul 30, 2023

been having same issues, looks like #3834 resolved it.
however, each save accumulates callback, which may cause issues:
image

const httpTerminator = createHttpTerminator({ server, gracefulTerminationTimeout });

const releaseResources = async () => {
  if (dataSource.isInitialized) {
    try {
      await dataSource.destroy();
      logger.warn('successfuly closed typeorm connection to database');
    } catch (error) {
      logger.error('error when closing database connection', error);
    }
  }
  await httpTerminator.terminate();
  logger.warn('closed http server');
};

const attemptGracefulShutdown = async (signal: string) => {
  logger.warn(`Attempting to gracefully shutdown for ${signal}`);
  await releaseResources();
  process.exit(0);
};

const signals = ['SIGTERM', 'SIGINT', 'SIGUSR2'];
signals.forEach((signal) => process.on(signal, () => attemptGracefulShutdown(signal)));

if (import.meta.hot) {
  import.meta.hot.on('vite:beforeFullReload', releaseResources);
  import.meta.hot.dispose(releaseResources);
}

@sheremet-va
Copy link
Member

sheremet-va commented Aug 14, 2023

#3834 is released as part of 0.34.0. vite-node now waits until all callbacks are resolved before reloading.

@GuskiS
Copy link

GuskiS commented Aug 14, 2023

@sheremet-va Is the issue that @scarf005 mentioned also fixed? Looking at his screenshot it seems that event callback is not being removed from events map. 🤔

@scarf005
Copy link
Contributor

@sheremet-va i've updated to v0.34.1, however the error seems to persist even without process.on hooks.

2023-08-14.17-42-09.mp4

@sheremet-va
Copy link
Member

Please, create a new issue with reproduction

@xuxucode
Copy link
Contributor

xuxucode commented Aug 14, 2023

@github-actions github-actions bot locked and limited conversation to collaborators Aug 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

9 participants