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

POST requests not working #78

Closed
sinedied opened this issue Dec 19, 2019 · 11 comments
Closed

POST requests not working #78

sinedied opened this issue Dec 19, 2019 · 11 comments

Comments

@sinedied
Copy link
Contributor

I'm submitting a...


[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

When using the npm run start:azure entry point or deploy my app on Functions, POST requests are not working. Using the regular entry point npm start, requests works well.

Expected behavior

POST requests works the same using azure functions entry points or the regular one.

Minimal reproduction of the problem with instructions

  1. Clone this project example project: https://github.com/nitro-stack/nitro-cats/blob/master/server/src/cat/cat.controller.ts
  2. npm start:azure
  3. upload an image file using POST /api/upload
  4. NestJS endpoint handler is never called

But really, this can be reproduced with any project and POST requests. (GET requests works fine)

What is the motivation / use case for changing the behavior?

Environment


Nest version: 6.10.12

 
For Tooling issues:
- Node version: 12.13.0  
- Platform:  Mac

Others:

@sinedied
Copy link
Contributor Author

I forgot to mention that when I say "not working", the request gets stuck with no response at all and timeout.
From the logs I can see the request being received by the functions emulator when running locally from the logs, but it doesn't seem to get forwarded to the "real" NestJS handler as I see no logs from my controller handler.

@sinedied
Copy link
Contributor Author

Ok, I did some extensive debugging this morning and found the cause of the issue: the functions runtime already parse the JSON body and provide it to the req object, but the underlying express layer tries to parse the body agains and gets stuck decoding the input stream, surely because the functions runtime already exhausted it. Did a quick workaround by adding req._body = true in the Azure http handler before giving the hand to the NestJS handler and that fixed the issue.

Though I willing to open a PR to fix this, I would like some advice on what you think would be the best approach to solve this? I'm also wondering about the upstream package azure-express-functions which I guess should have the issue too?

@sinedied
Copy link
Contributor Author

Sorry for the spam, seems a workaround has already been proposed to azure-function-express, but is still not merged after many months: yvele/azure-function-express#31

@sergfa
Copy link

sergfa commented Jan 3, 2020

Thanks @sinedied, your workaround solved this problem in my env.
I'm posting a code snippet so others can use it until the fix will be merged.

// main/index.ts

export default function(context: Context, req: HttpRequest): void {
  req['_body'] = true;
  AzureHttpAdapter.handle(createApp, context, req);
}

@kamilmysliwiec
Copy link
Member

Thanks for sharing @sinedied. I've added this workaround to the AzureHttpAdapter and published as a new version.

@sinedied
Copy link
Contributor Author

sinedied commented Jan 7, 2020

Thanks @kamilmysliwiec , but to be honest this workaround was not meant for production as it doesn't solve all issues here, for example file upload are still not working.
In fact, any other express middleware other than express/body-json will still be failing, unless that PR: yvele/azure-function-express#31 is merged.

I can't find any way to contact the original author of azure-function-express as he doesn't respond on github and have its DM closed on twitter. There's no sign of recent activity from him on this project since almost a year 😢
I'm tempted to fork the project, merge the bugfix PRs and publish a new package on NPM to unblock the issue as it's quite problematic.
Would you accept a PR to (temporarily) use that fork to address the issue?

Meanwhile I'm still trying to contact the original author, as the best fix would still best to be upstream.

@kamilmysliwiec
Copy link
Member

Would you accept a PR to (temporarily) use that fork to address the issue?

@sinedied maybe we should no longer use the "azure-function-express" and just bring the entire functionality here? Hence, we won't be dependent on them in the future. This package seems to be fairly small either way. Would you like to contribute to this and (if so) create a PR?

@sinedied
Copy link
Contributor Author

sinedied commented Jan 8, 2020

@kamilmysliwiec even better, I'll work on that. Do you want to reopen this issue or should I open a new one to track the changes?

@kamilmysliwiec
Copy link
Member

I would suggest creating a new one:)

@dcollien
Copy link

@sinedied I shared your frustration regarding azure-function-express and have moved my own dependencies over to use https://github.com/mikestaub/serverless-express (added benefit of it now being deployable on 3 clouds)

@jpgilchrist
Copy link

@dcollien I'm interested in that too, I saw your PR there for the stream IncomingMessage fix. This thread is probably not the best place for this conversation, but I don't have a better spot for it. Do you have a mini guide on migrating from azure-functions-express to serverless-express?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants