Comments (14)
You need to abort your I/O yourself and keep track of when you need to call res.send
so you don't do it a second time. You do this by listening for the timeout
event on req
.
from timeout.
honestly, timeouts are always goign to be a pain in node. you'll have to handle it yourself like @dougwilson said, but some code would be nie ot figure out what's going on.
you could also do req.setTimeout()
. this way, the socket will just be closed when it times out, but then you won't be able to send a pretty error message or page.
from timeout.
Another trick is to make timeout middleware that no-ops .send and.end etc
after you respond. Many tricks.
On Mar 4, 2014 2:56 PM, "Jonathan Ong" [email protected] wrote:
honestly, timeouts are always goign to be a pain in node. you'll have to
handle it yourself like @dougwilson https://github.com/dougwilson said,
but some code would be nie ot figure out what's going on.you could also do req.setTimeout(). this way, the socket will just be
closed when it times out, but then you won't be able to send a pretty error
message or page.Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-36666742
.
from timeout.
This could also be reproduced by combining timeout
and bodyparser
. When it times out before the bodyparser
gets the end
event from the incoming stream (very common when bandwidth is fully occupied), proto.js
in connect
will always throw.
Verified on both express 3.4.0 and 3.4.8.
Stack trace on express 3.4.8:
Error: Can't set headers after they are sent.
at ServerResponse.OutgoingMessage.setHeader (http.js:691:11)
at ServerResponse.res.setHeader (./node_modules/express/node_modules/connect/lib/patch.js:63:22)
at next (./node_modules/express/node_modules/connect/lib/proto.js:156:13)
at multipart (./node_modules/express/node_modules/connect/lib/middleware/multipart.js:93:27)
at ./node_modules/express/node_modules/connect/lib/middleware/bodyParser.js:64:9
at ./node_modules/express/node_modules/connect/lib/middleware/urlencoded.js:74:7
at null._onTimeout (./node_modules/express/node_modules/connect/node_modules/raw-body/index.js:109:7)
at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)
from timeout.
This module will not do anything after the headers have been sent, so there is nothing this module can do. The stack trace in this issue doesn't even lead to this module, which means if there is some other place that is setting a header after headers have been sent, it needs to be fixed there.
from timeout.
It will call next().
I opened a bug months ago against connect to handle such situations more gracefully, but it's closed as you thought it would be better fixed here. Now two bugs are both closed....
from timeout.
It will call next().
Are you using node.js 0.8?
from timeout.
No, it's 0.10.
from timeout.
OK. So, if you look in this module's source code, you can see that on timeout, it will not call next()
after the headers have already been sent. This means this module is doing everything it can to not send after headers. From your trace above, it shows that you are the one sending the response back, not this module. When you use this module, you need to listen to req.on('timeout', function(){...})
and stop your stuff from responding.
Your stack trace shows that connect/lib/proto.js:156:13
is what is trying to set the header incorrectly. It looks like this is the default 404 handler within connect
2.12.0. This looks like the 404 is what is in the wrong here...
I'm re-opening your connect
bug and it'll be fixed in the next connect
2.x/express
3.x combo release.
Sorry it took this long to figure out, but we didn't have a test case to reproduce it locally to figure it out. This still may not really be what your issue is, as without a test case from you to run, I cannot know for sure.
from timeout.
@dougwilson Actually timeout happens first, and no headers are sent yet. So timeout
will call next(err)
, causing a 500 response sent in proto.js
. Sometime later, the end
event gets fired on the underlying socket, and it finally goes into the 404 handler.
Fix the 404 bug should do the trick.
But the fact that timeout
module calls the same next()
twice in this scenario: first in line 53 and second in line 42 might not be supported by the connect
design, according to my understanding of the code. That's why it finally goes to 404 part.
from timeout.
the fact that timeout module calls the same next() twice in this scenario
This is not a bug, it is just what this module does. You have to call next()
the first time, otherwise the request will never eave this middleware and all your requests will timeout.
If you have not yet responded and so the request timesout, this module will respond on your behalf, thus the second next(err)
call. You need to stop your own stuff from responding after this module has responded. The default 404 in connect
was in the wrong in your example, and it has been fixed.
The other thing to note about eventemitters is that you need to add your req.on('timeout')
listener immediately after this middleware. So if you are doing
app.use(express.timeout(5000))
app.use(express.bodyParser())
app.use(...)
you are going to be hurting. You need to add your custom req.on('timeout')
listener before the bodyParser in that example. Besides that, the layout shown above is easy, but terrible, as you can see in your own stack trace you are parsing some request body... just to then 404, which was a waste of your server's CPU time.
If you want to keep using these middlewares in this (i.m.o. bad) configuration, just do this:
app.use(express.timeout(5000))
app.use(function(req, res, next){
req.on('timeout',function(){
// pretend like data was written out
res.write = res.end = function(){ return true };
// no headers, plz
res.setHeader = res.writeHead = res.addTrailers = function(){};
});
next();
})
app.use(express.bodyParser())
The above is a last resort, may cause issues, because it's trying to override too much, etc., so it'll never make it into this module. This module is already doing the correct behavior, it is simply that you are not integrating it into your application correctly. The connect
404 handler should have never been called in the first place, because you were responsible for stopping the flow after the timeout middleware, but because of the way you are using it (as a top-level middleware) you are just asking for pain.
from timeout.
FYI, if you are wonder what the "correct"way to use middleware and this module is, here is a simple express 3.x example:
var express = require('express');
var app = express();
app.post('/upload', express.timeout(5000), express.bodyParser(), function(req, res, next){
if (res._header) return; // someone already responded
var timedout = false;
req.on('timeout',function(){
timedout = true;
});
// pretend setTimeout is something long, like uploading file to s3
setTimeout(function(){
if (timedout) return; // timedout, do nothing
res.send('file uploaded!');
}, 6000); // adjust meee
});
app.listen(3000);
you'll notice the non-use of top-level middleware and the tracking of if your request timedout during your long operation so you can not send back a response.
from timeout.
@roamm please check out the Readme for the intended way to use this module now. The stuff in the Readme requires 1.1.0 of this module to function.
from timeout.
I've read these and the Readme, that's exactly the problem. I'll change the way I use it. Thank you for explaining these.
from timeout.
Related Issues (20)
- Can't set headers after they are sent. HOT 1
- clearer documentation HOT 4
- 'connect-timeout' is possibly misleading HOT 2
- How to change timeout response? HOT 1
- How to catch "Can't set headers after they are sent." HOT 2
- Make this configurable via middleware.json HOT 1
- Extending the timeout HOT 7
- ServiceUnavailableError: Response timeout HOT 2
- "Can't set headers after they are sent" error crashes the app HOT 1
- Documentation Update HOT 4
- Yarn certificate has expired. HOT 1
- What is the associate between this library and http's timeout HOT 1
- Should remove listener to timeout event HOT 7
- Turning off response timeouts HOT 2
- Make timeout action more flexible HOT 4
- Source code was not found HOT 4
- Override timeout for a route HOT 3
- req.timedout always come out to be false irrespective of timeout happens or not HOT 11
- `timedOut` instead of `timedout`?
- Upgrade Dependencies
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from timeout.