Requests stuck in a pending state when using a Interceptor and node 14

[DiFuks]

// AppModule.ts

@Module({
  imports: [
    RenderModule.forRootAsync(
      Next({
        dev: process.env.NODE_ENV !== 'production',
        dir: path.resolve(__dirname, '../..'),
      }),
    ),
  ],
  controllers: [IndexController]
})
export class AppModule {}

// controllers/IndexController.ts

@Controller()
export class IndexController {
  @UseInterceptors(new FormatResponse('Index'))
  @Get('/')
  public get(): void {}
}

// interceptors/FormatResponse.ts

@Injectable()
export class FormatResponse implements NestInterceptor {
  public constructor(private readonly pageFileName: string) {}

  public intercept(
    context: ExecutionContext,
    next: CallHandler,
  ): Observable<unknown> {
    const response: Response = context.switchToHttp().getResponse();

    return next
      .handle()
      .pipe(map((payload) => response.render(this.pageFileName, payload)));
  }
}

After a request to the address http://localhost:3000/ the server serves all files except one. This file gets stuck in "pending" status. IMPORTANT! The bug is only seen in 14+ versions of node.js. There is no error in version 12:

[DiFuks]

By direct request in the address bar http://localhost:3000/_next/static/chunks/main.js?ts=1605799518477 the file is successfully loaded

[DiFuks]

This link has a similar problem https://stackoverflow.com/questions/63610932/express-static-network-requests-stuck-on-pending

[kyle-mccarthy]

have you tried checking if it works correctly on node 12?

[DiFuks]

@kyle-mccarthy Yes, in the description of the issue I wrote:

There is no error in version 12

This is a minimal version of the project with a reproduced issue: https://github.com/DiFuks/nest-next-minimal

For a successful run:

1. nvm use 12
2. npm run dev

or

1. nvm use 14
3. npm run prod

For an unsuccessful run:

1. nvm use 14
3. npm run dev

[DiFuks]

Perhaps the problem is not with your library, but for example with express. But first I would like to make sure of this

[DiFuks]

This hard fix worked for me: https://github.com/DiFuks/nest-next-minimal/commit/e8fb00c503ac65f3fbfc24d402cc8d47b79a9f1c

[kyle-mccarthy]

Okay I've spent the better part of the last 2 days debugging this and think I have an idea of what's going on. However, I'm not entirely sure what is responsible for the problem. The primary thing is that the interceptor seems to be causing the next incoming http request to hang. You can verify this by using node14 and the Render decorator on the controller's method.

I suspect that the problem is caused by (a) changes to the event loop in node 14 or (b) changes to ending or closing the http response. The reason that I think this is that I can actually make the interceptor work by forcing the map operation to resolve after an two ticks on the process rather than just the single tick. This indicates that there is some race condition at play but I haven't been able to determine the source of the condition.

@Injectable()
export class FormatResponse implements NestInterceptor {
  public constructor(private readonly pageFileName: string) {}

  public intercept(
    context: ExecutionContext,
    next: CallHandler
  ): Observable<unknown> {
    return next.handle().pipe(
      map(async (payload) => {
        const response = context.switchToHttp().getResponse();

        await response.render(this.pageFileName, payload);

        await new Promise<void>((resolve) => {
          return process.nextTick(resolve);
        });

        return;
      })
    );
  }
}

By making the map function return the promise which requires an extra tick to resolve it keeps the the next request from hanging. I actually suspect that by using readFileSync in your fix above supports my race condition theory. The function call blocks the event loop allowing for whatever is causing the problem to finish running.

Let me know what you think, I do think that this is an upstream issue unrelated to this repo but nonetheless is pretty interesting.

[DiFuks]

This solution works for me. But I don't understand why response.render is returning a promise.
What do you think is the best way to address this problem? To the node.js repository?

[DiFuks]

I'm afraid my knowledge of node.js and English is not deep enough to correctly formulate the problem. 😅

[kyle-mccarthy]

The response.render is returning a promise because it is actually calling next's render function instead of the express render function. This library overrides the default render function provided by express with next's render function. This is primarily done to allow for devs to easily render without directly using the Render decorator or fetching the RenderService from the IoC container.

I am not exactly sure of the best path forward since there are many different package at play (this one, next, nest, express, etc..) so it's difficult to determine what's responsible for the change. For instance, it could be that node 14 is a little faster than node 12 exposing the race condition. If this is something that you want to dive deeper into, you will probably want to use node's debugger and isolate differences between how v12 and v14 handle the problematic request.