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

NestJS 9 with Fastify ignores @Res() in controller #10333

Closed
2 tasks done
coryroloff opened this issue Sep 30, 2022 · 7 comments
Closed
2 tasks done

NestJS 9 with Fastify ignores @Res() in controller #10333

coryroloff opened this issue Sep 30, 2022 · 7 comments
Labels
needs triage This issue has not been looked into type: bug 😭

Comments

@coryroloff
Copy link

Did you read the migration guide?

  • I have read the whole migration guide

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Potential Commit/PR that introduced the regression

No response

NestJS version

8.4.7 -> 9.1.2

Describe the regression

NestJS appears to be ignoring the use of @res() to override Nest's standard code for responding to a request, specifically when using Fastify.

In version 8, Nest doesn't attempt to respond to requests at all if it sees @res() in the controller's parameters and allows you to respond when needed (for example, any asynchronous code not through a Promise). In version 9, the same code will respond immediately, ignoring your res.send if it doesn't happen synchronously.

Note: NestJS 9 using Express behaves the same way as NestJS 8 using Fastify.

Minimum reproduction code

https://github.com/coryroloff/nestjs-res-bug

Input code

No response

Expected behavior

To test NestJS 8, run the following:

cd nestjs-8
npm install
npm run start:dev

curl localhost:3000/test-res-bug -v

You will get the response "hello world" back. This is expected behavior.

To test NestJS 9 with Fastify, run the following:

cd nestjs-9
npm install
npm run start:dev

curl localhost:3000/test-res-bug -v

You will get an empty response body back, instead of the expected "hello world".

To test NestJS 9 with Express, run the following:

cd nestjs-9-express
npm install
npm run start:dev

curl localhost:3000/test-res-bug -v

You will get the response "hello world" back. This is expected behavior.

Other

No response

@coryroloff coryroloff added needs triage This issue has not been looked into type: bug 😭 labels Sep 30, 2022
@jmcdo29
Copy link
Member

jmcdo29 commented Sep 30, 2022

This actually might be related to something I'm looking into with @fastify/view integration and not being able to res.view() from an exception filter. I think there's somewhere that an await is possibly missing that's causing fastify to send a response before we expect it to. I'll try to update this as I can

@kamilmysliwiec
Copy link
Member

Did you have a chance to find the root cause @jmcdo29? Let me know and I can take over if you're busy 🙌

@SirReiva
Copy link
Contributor

image
This may be the problem

@jmcdo29
Copy link
Member

jmcdo29 commented Oct 25, 2022

Did you have a chance to find the root cause @jmcdo29? Let me know and I can take over if you're busy raised_hands

@kamilmysliwiec that might be best. What SirReiva added might be relevant here too

@coryroloff
Copy link
Author

@SirReiva's answer explains and solves the issue I was running into. Thanks, everyone.

@SirReiva
Copy link
Contributor

image
making this change apparently works

@kamilmysliwiec
Copy link
Member

Let's track this here #10459

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into type: bug 😭
Projects
None yet
Development

No branches or pull requests

4 participants