Nest: Nest exits silently if app.listen() is given invalid IP address

[dolsem]

Bug Report

Current behavior

Currently if Nest is given an IP address that the server cannot listen on with any of the existing network interfaces, the app starts and then silently exits with no error message being thrown. This makes it very difficult to track down the issue.

Input Code

async function bootstrap() {
  const app = await NestFactory.create<NestFastifyApplication>(
    AppModule,
    new FastifyAdapter(fastify({ logger })),
  );
  const config = app.get(ConfigService);
  await app.listen(config.port, config.address);
}
bootstrap();

Expected behavior

An error should be thrown that informs the user that the server cannot bind to this address + port.

Environment

Nest version: 6.5.3


For Tooling issues:
- Node version: v12.10.0
- Platform:  Linux

[izio38]

A promise should be followed by a catch calls.

Also I don't want my entire nest app crash when the listen didn't work.

He should just not launch the http server instance, what it actually does.

Le jeu. 17 oct. 2019 à 16:23, Denis Olsem [e-mail] a
écrit :

Bug Report Current behavior

Currently if Nest is given an IP address that the server cannot listen on
with any of the existing network interfaces, the app starts and then
silently exits with no error message being thrown. This makes it very
difficult to track down the issue.
Input Code

async function bootstrap() {
const app = await NestFactory.create(
AppModule,
new FastifyAdapter(fastify({ logger })),
);
const config = app.get(ConfigService);
await app.listen(config.port, config.address);
}bootstrap();

Expected behavior

An error should be thrown that informs the user that the server cannot
bind to this address + port.
Environment

Nest version: 6.5.3

For Tooling issues:

• Node version: v12.10.0
• Platform: Linux

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/nestjs/nest/issues/3209?email_source=notifications&email_token=ACLRGLUP5MEYA7JIGO2NEQDQPBYPPA5CNFSM4JB2NCUKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HSPI5ZQ,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ACLRGLTCT6ZMEFMGEPKLEKTQPBYPPANCNFSM4JB2NCUA
.

[dolsem]

@izio38

A promise should be followed by a catch calls.

You mean change bootstrap() to bootstrap().catch(err => console.error(err))? I've already tried doing that, it still doesn't output the error. In fact, if there was an error thrown in bootstrap(), I wouldn't have created the issue, because Node would log uncaught promise rejection. Right now nothing gets thrown.

He should just not launch the http server instance, what it actually does.

You might be right, but I think in that case Nest should still output a warning, or indicate in some other way that the listen() call didn't work.

[jmcdo29]

@dolsem the problem here is that the underlying Node HTTP server's listen doesn't throw an error, but instead uses callbacks. Maybe an error handler could be added here for the case of a bad port and hostname. change it to something like

public listenAsync(port: number | string, hostname?: string): Promise<any> {
  return new Promise((resolve, reject) => {
    const server: any = this.listen(port, hostname, (err) => err ? reject(err) : resolve(server));
  });
}

Even Express doesn't throw an error when you provide a bad port according to their code. Could be a good improvement though

[dolsem]

@jmcdo29 I looked into your suggestion and found out that Nest's listen() function does accept callback as its third argument. This callback does indeed return an error with code EADDRNOTAVAIL, though its TS type definition is incorrect (function with no arguments).

Personally I find this API counter-intuitive, since this is an async method. I think listen method promise should reject with this error (which can then obviously be caught and handled by the user).

[jmcdo29]

I believe the only reason the listen method is marked as async is to allow for the await this.init() function if it needs to be called. Maybe it makes more sense to return a promise in the listen method and remove listenAsync as they both should return the same thing (a promise of the httpServer). And yeah, looks like the callback function's parameters should be updated.

[kamilmysliwiec]

Good point @jmcdo29. Would you like to create a PR for this issue @dolsem?

[dolsem]

@kamilmysliwiec sorry for a late response. I'd be happy to create a PR, but I've been very busy lately. Might get to it in the next week or so.

[Sikora00]

Is that issue still available to take? I could take that.

[dolsem]

@Sikora00 the PR attached above is fixing this issue