search

restify / clients

HttpClient, StringClient, and JsonClient extracted from restify
MIT License
57 stars 34 forks source link

fix: Fix bug where client callback is never called if server closes c… #239

Closed daviande closed 2 years ago

daviande commented 2 years ago

…onnection mid response

wesleytodd commented 2 years ago

LGTM. Just for comparison in Express this is the module used to do this exact same thing: https://www.npmjs.com/package/ee-first

Not sure if there are any differences, but might be worth checking for: https://github.com/jshttp/on-finished/blob/master/index.js#L115

I think the only thing I can think of is the cleanup to remove the events which I don't see happening here. Ex: https://github.com/jonathanong/ee-first/blob/master/index.js#L60-L66

Since the res is GC'd I wouldn't expect it to be an issue, but most of the express code is pretty hardened even against the weirdest of cases, so often good to follow.