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

'end' or 'close' event or callback for response #6

Closed
jzaefferer opened this issue Jan 21, 2013 · 8 comments · Fixed by #11
Closed

'end' or 'close' event or callback for response #6

jzaefferer opened this issue Jan 21, 2013 · 8 comments · Fixed by #11

Comments

@jzaefferer
Copy link

The example in the readme assumes that the response is sync. Since I'm testing a wrapper around an actual http API, the response is async, so I need an event or callback. Neither "end" nor "close" seem to get emitted, and I don't think I'm missing something, since the end method doesn't bother.

I can provide a patch, but would like to get confirmation that it would get merged etc.

@GiordanoArman
Copy link
Contributor

@eugef I noticed that the response.end() callback you find in the official Node.js docs

https://nodejs.org/docs/latest-v12.x/api/http.html#http_response_end_data_encoding_callback

is not triggered. Is it possible to have the mock response object mimick the native response on this matter?

@eugef
Copy link
Owner

eugef commented Aug 19, 2021

Hi @GiordanoArman PR is always welcome.

Meanwhile you can manually trigger response.end() callback in the tests this way:

import httpMocks from 'node-mocks-http';

const req = httpMocks.createRequest();
const res = httpMocks.createResponse();
const next = () => {};

someMiddleware(req, res, next);

res.end();

@GiordanoArman
Copy link
Contributor

@eugef ok, thanks, would you rather have this fix on a fork of the 1.x branch or master branch?

@eugef
Copy link
Owner

eugef commented Aug 23, 2021

Master branch

@GiordanoArman
Copy link
Contributor

ok, great :) will this fix be released in a 1.x version or when you release version 2?

@eugef
Copy link
Owner

eugef commented Aug 23, 2021

I assume this feature won't introduce a breaking change, so it would be possible to release as a minor version.

@GiordanoArman
Copy link
Contributor

ok, do you perhaps need to add me as collaborator somehow in order for me to push the changes? I'm stuck with auth errors, whether I use personal tokens or SSH keys... been a while since last time I used GitHub via command line lol

@eugef
Copy link
Owner

eugef commented Aug 24, 2021

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 a pull request may close this issue.

3 participants