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

Missing request data events #6

Closed
silentjohnny opened this issue Jun 18, 2013 · 5 comments
Closed

Missing request data events #6

silentjohnny opened this issue Jun 18, 2013 · 5 comments

Comments

@silentjohnny
Copy link

Is there a reason for the request.pause() / resume() calls? Because (in node 0.10) request handlers now miss data events. Removing the pause/resume from the cors module resolves the problem.

Any thoughts on this?

@troygoode
Copy link
Member

This was actually implemented specifically so that data events wouldn't go missing, if someone was using the async version of the cors options setup. In older versions of node.js (prior to 0.10) .pause() and .resume() were necessary to prevent loss of data events of a stream between ticks.

Are you still running into this problem?

@Floby
Copy link

Floby commented Apr 25, 2014

I'm running into this problem as well

I'm using this module for static/sync cors headers before proxying to a couchDB url but it still consumes my request. I can understand the pause() calls, but I have a harder time understanding why resume() is called.

Anyway, this is not the recommended way of doing things in node >=0.10. pause() and resume() cause the request to switch to old streams mode and thus consuming the stream before I get a chance to use it. Most importantly, it causes this fix (nodejs/node-v0.x-archive#4969) to be unefficient.

Since i'm using request to do one-line proxying, I have some lines like this down the middleware chain.

req.pipe(request(url)).pipe(res)

since req is already consumed (and has already emitted 'end'), it causes my service to hang on this particular request. (yes i'm piping req even though it's a GET request, but this way request can use the clients cache-control headers and pass them through).

I can see several fixes for this.

  1. don't call pause()/resume() when cors is in sync mode
  2. don't call pause()/resume() at all, since it's hardly cors responsibility to worry about the body of a request
  3. only use streams2 API.

my fix for my own problem for now is to remove cors altogether and set my static header myself =)

@troygoode troygoode reopened this Apr 25, 2014
@troygoode
Copy link
Member

Let me look into this.

@troygoode
Copy link
Member

I've removed pause and resume and specified 0.10 as the minimum required version of node.js.

@Floby
Copy link

Floby commented Jul 6, 2014

Thanks!
Le 5 juil. 2014 20:48, "Troy Goode" notifications@github.com a écrit :

I've removed pause and resume and specified 0.10 as the minimum required
version of node.js.


Reply to this email directly or view it on GitHub
#6 (comment).

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

3 participants