Skip to content

Instantly share code, notes, and snippets.

@mikedeboer
Created July 4, 2012 12:31
Show Gist options
  • Star 2 You must be signed in to star a gist
  • Fork 2 You must be signed in to fork a gist
  • Save mikedeboer/3047099 to your computer and use it in GitHub Desktop.
Save mikedeboer/3047099 to your computer and use it in GitHub Desktop.
Why to call resume() after next()

When we write Connect middlewares that contain async operations, it is good practice to 'pause' the HTTP request with the help of the Connect utility function. When the async operation has finished, the HTTP request is resumed again. Let's have a look at the documentation for this utility:

Pause `data` and `end` events on the given `obj`.
Middleware performing async tasks _should_ utilize
this utility (or similar), to re-emit data once
the async operation has completed, otherwise these
events may be lost.

What this means is that during the time the HTTP request is paused, end and data events are buffered and immediately replayed when pauseControl.resume() is called. On to explaining WHY the order matters as when to resume the HTTP request:

// WRONG CODE!!!
var pause = utils.pause(req);
fs.readFile(path, function(){
    pause.resume();
    next();
});

The next function invokes the next middleware on the stack of the Connect server. This next middleware may start listening to data and end events as well, which makes it likely that it would like to receive the buffered data and end events as well that were captured during the pause. Since the resume() is invoked BEFORE the next middleware, the data and end events have already been replayed, thus missed by the event listeners that will be set inside the next middleware. The result is unexpected behavior for the middleware layers after this one, because there is a chance that the end may not be emitted anymore, resulting in never-ending HTTP request (because the server will only start sending back the response when it received and parsed the request data).

@creationix
Copy link

Yes, I'm afraid this is one of the ugly edges in the connect API I designed. Thanks for the writeup. And I'm glad there is a utility for this. Also the native Stream pause and resume should do this in future node releases.

@fjakobs
Copy link

fjakobs commented Jul 6, 2012

So I blame you for the bug I introduced :)

@creationix
Copy link

I never said that introducing connect to the world was a good thing. But what's done is done, no taking it back now. At the time it was very much needed, there was serious fragmentation in node http libraries and nothing could be shared. The (req, res, next) API was very simple and worked well for 85% of the cases.

@fjakobs
Copy link

fjakobs commented Jul 6, 2012

It was a good thing.

@Asimov4
Copy link

Asimov4 commented Feb 17, 2014

This is a test comment.

@Asimov4
Copy link

Asimov4 commented Feb 17, 2014

This is a test comment.

@Asimov4
Copy link

Asimov4 commented Feb 17, 2014

This is a test comment.

@Asimov4
Copy link

Asimov4 commented Feb 17, 2014

This comment has been edited.

@NorikDavtian
Copy link

This is a test comment.

@NorikDavtian
Copy link

This is a test comment.

@NorikDavtian
Copy link

This is a test comment.

@NorikDavtian
Copy link

This comment has been edited.

@NorikDavtian
Copy link

This is a test comment.

@NorikDavtian
Copy link

This is a test comment.

@NorikDavtian
Copy link

This is a test comment.

@NorikDavtian
Copy link

This comment has been edited.

@NorikDavtian
Copy link

This is a test comment.

@NorikDavtian
Copy link

This is a test comment.

@NorikDavtian
Copy link

This is a test comment.

@NorikDavtian
Copy link

This comment has been edited.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment