Skip to content

No longer warns on res.close event#22

Merged
tellnes merged 1 commit intotellnes:masterfrom
msmol:master
Jul 29, 2019
Merged

No longer warns on res.close event#22
tellnes merged 1 commit intotellnes:masterfrom
msmol:master

Conversation

@msmol
Copy link
Copy Markdown

@msmol msmol commented Jun 1, 2018

See: nodejs/node#20611

res.close is always triggered and is no longer considered and error or warning condition

Addresses: #21

See: nodejs/node#20611

res.close is always triggered and is no longer considered and error
or warning condition
@mareksrom
Copy link
Copy Markdown

I think that change in #20611 is incosistent with the documentation of close event - "Indicates that the underlying connection was terminated before response.end() was called or able to flush." - this is not true anymore - event close is called always. Either the documentation should be changed or this PR should be reverted or fixed. I don't know if it is intended behaviour (then isn't it a API change - SEMVER-MAJOR?), but from my point of view only unintended side efect of the fix. Previous functionality can be achieved by testing res.finished in close event...

@msmol
Copy link
Copy Markdown
Author

msmol commented Jul 8, 2018

@mareksrom see nodejs/node#21047

@eiabea
Copy link
Copy Markdown

eiabea commented Aug 22, 2018

@tellnes any plans to merge this PR? 🙏

@michaltk
Copy link
Copy Markdown

This doesn't seem to be getting merged so forked and fixed.
https://github.com/michaltk/bunyan-middleware

@tellnes tellnes merged commit e86dc99 into tellnes:master Jul 29, 2019
@tellnes
Copy link
Copy Markdown
Owner

tellnes commented Jul 29, 2019

Sorry for the delay, merged. I'll publish to npm 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.

5 participants