-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Can't set headers after they are sent.] Error: Can't set headers after they are sent. 2.2.18 #2580
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
Comments
My workaround is never putting any route after /parse route even the 404 route .Put all the other routes before /parse instead, for 404 just exclude /parse |
I think this happens when our client saves a userTag, stuff they tag in our app.. This happens on a few of our queries for POST from our app..
|
Did you try to update express to latest version? |
@GuichiZhao Would you pls show me your workaround code? |
@anhldbk
cloud/routes/index.js
|
@GuichiZhao Thanks. |
@flovilmart When will your patches available in |
Same here. express 4.14.0 |
The fix (that will probably land in 2.2.19) is intended to fix the internal problems with 'Cannot set headers' errors. For you, that use a custom express installation, you should stick with the workaround. |
@kronos so updating to 4.14.0 fixed this for you? |
@mcclaskiem No. I meant that the problem is reproducible with the latest express |
Hey guys, I wanted to upgrade to 2.2.18 for the mongo connection improvements and ran into this issue as well. After digging into it I found that it was because of #2338 (this commit). The change was added to allow "developers to add express handlers for successful completion of parse-server requests" however this causes problems for anyone who uses Parse Server in an existing Express app like this: Why it is breaking is that by adding the The correct workaround for doing this atm is to add a callback that stops the propagation: app.use('/parse', new ParseServer({...}), function(req, res, next) {
// This will get called after every parse request
// and stops the request propagation by doing nothing
}); I don't think 👍 |
I ageee with your workaround, and it should be added onto the Parse-server-example repository (PR are welcome). When using though the CLI / standalone, we should not yield any headers already sent, internally, as we manage our routes. On another note, calling next after the response is sent is also an opportunity to cleanup stuff (like caches) after the response is sent. It's also an opportunity to do request tracing, logging etc.. (as it's been intended by the commit you point out). There is nothing in the express design that prevents having 'post' middlewares, and they can be quite useful. For now, it's been decided that next call was here to stay, there are numerous was to work around that. On another note, adding pre middlewares can have even worse side effects, like a json body parser would conflict with the files endpoint. |
This is not a middleware though (by definition something that sits inbetween the start of the request and the handler). This IS the handler. This makes routing messy. In node if you wanted to do things like request tracing & logging, wouldn't you generally do the following: app.use('/parse', function(req, res, next) {
// Do PRE work
next();
// Do POST work
}); In our case it would be: var parse = new ParseServer({...});
app.use('/parse', function(req, res, next) {
req.on('response', function() {
// Track anything on response, such as resp time
});
// Manually invoke the parse server router
parse(req, res, next);
// Immediate POST tracing etc
}); I disagree but fair enough 👍 I shall keep my hack in place |
Definition of middleware is debatable as well, take django, middlewares are traversed in and out, before and after the request is processed... @blacha, what do you think? Seems that this small commit is causing some ire. |
haha but my definition sounded so totally technical :P |
I would also prefer to drop the call to next() after responding now that there is a workaround provided for logging. It does seem that event handlers are the way to go with this and are an app-specific solution rather than something that should be built in to parse-server. While it does the job, the fix in #2559 requires checking overlapping routes within route handlers so the routes are no longer as clearly defined. This fix wouldn't be required if we dropped next(). |
Waiting to hear on @blacha's take on that as he provided that. |
I had this error today when I upgrade to node to 6.4.0 then I tried to downgrade back to 4.5 but still get the same error.
|
Fixed with 2.2.19 |
We've added unit tests to make sure the next() handlers are never yieled, probably you have a next(err) handler |
Please read the following instructions carefully.
Check out #1271 for an ideal bug report.
The closer your issue report is to that one, the more likely we are to be able to help, and the more likely we will be to fix the issue quickly!
Many members of the community use Stack Overflow and Server Fault to ask questions.
Read through the existing questions or ask your own!
For database migration help, please file a bug report at https://parse.com/help#report
Make sure these boxes are checked before submitting your issue -- thanks for reporting issues back to Parse Server!
Issue Description
While looking into a cloud code issue which I fixed, I saw this issue in my Heroku logs and Parse Dashboard logs.. I have no idea what this could be but doesnt sound to good.
Not sure how to debug this, but needed to provide this to the community.
I should note rolling back to 2.2.17 this does not happen.. :/
Environment Setup
The text was updated successfully, but these errors were encountered: