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

Fix issue #159 #162

Merged
merged 2 commits into from
Feb 8, 2012
Merged

Fix issue #159 #162

merged 2 commits into from
Feb 8, 2012

Conversation

dpetukhov
Copy link

  1. Request freezes when remote server send headers and stops.
  2. Timeout not sets after redirecting request.
    https://github.com/mikeal/request/blob/master/main.js#L361 - on this line self.timeoutTimer stay assigned after redirect
    https://github.com/mikeal/request/blob/master/main.js#L487 - this line failed after redirect because self.timeoutTimer has value

Please tell me if you have additional requirements for accept this patch. Docs update, additional test, etc.

Regards,
Dmitry

@mikeal
Copy link
Member

mikeal commented Jan 23, 2012

only set the additional socketTimeout if the timeout option is set, otherwise it should use the default socket timeout which should propagate (as of 0.6.0) to the request object and we'll already handle it.

also, test to make sure that aborting the request doesn't already cause an error. if it does then we'll end up emitting double-errors.

@dpetukhov
Copy link
Author

This block inside if (self.timeout) and socket timeout will be applied only with current setTimeout mechanism.

About double error: I will check it.

@mikeal
Copy link
Member

mikeal commented Jan 23, 2012

Great, after you check on the double error comment and I'll merge.

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

Successfully merging this pull request may close these issues.

None yet

2 participants