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

The "end" event isn't emitted for some responses #35

Merged
merged 1 commit into from
Jun 9, 2011
Merged

The "end" event isn't emitted for some responses #35

merged 1 commit into from
Jun 9, 2011

Conversation

voxpelli
Copy link
Contributor

@voxpelli voxpelli commented Jun 8, 2011

The "end" event that was supposed to be emitted to fix a core bug in NodeJS wasn't fired because it wasn't emitted on the response object.

I encountered this problem when trying to fetch https://profiles.google.com/VoxPelli through JSDom. I tracked it down to being that the response object in this file only emitted "data"-events and no "end"-events. The change here fixed that.

(I hope the bonus spelling correction of "because" is okay as well :) )

…NodeJS wasn't fired because it wasn't emitted on the response object.


I encountered this problem when trying to fetch https://profiles.google.com/VoxPelli through JSDom. I tracked it down to being that the response object in this file only emitted "data"-events and no "end"-events. The change here fixed that.

(I hope the bonus spelling correction of "because" is okay as well :) )
@mikeal
Copy link
Member

mikeal commented Jun 8, 2011

do all the tests still pass?

@voxpelli
Copy link
Contributor Author

voxpelli commented Jun 9, 2011

Tested now and seems like they do

@smurthas
Copy link

smurthas commented Jun 9, 2011

I saw this same problem - attached code fixes it for me (with a GET to https://www.google.com/m8/feeds/contacts/default/full)!

@mikeal
Copy link
Member

mikeal commented Jun 9, 2011

i can't click that link.

mikeal added a commit that referenced this pull request Jun 9, 2011
The "end" event isn't emitted for some responses
@mikeal mikeal merged commit 4be60ee into request:master Jun 9, 2011
@smurthas
Copy link

smurthas commented Jun 9, 2011

That link was a link that demonstrates the problem. As in:

request({url:'https://www.google.com/m8/feeds/contacts/default/full'}, function(err, resp, body) {
    //never gets here!
});

Rolling a new npm version anytime soon? :)

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

3 participants