-
Notifications
You must be signed in to change notification settings - Fork 174
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 Server shutdown with "Error: Can't set headers after they are sent."
(close #937)
#940
Conversation
❌ Tests for the commit 0415c72 have failed. See details: |
0415c72
to
9202cba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\r-
@@ -112,8 +112,11 @@ export default class DestinationRequest extends EventEmitter { | |||
this._send(); | |||
} | |||
|
|||
else if (this._isDNSErr(err)) | |||
else if (this._isDNSErr(err)) { | |||
this._abort(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense to refactor whole code into method:
_fatalError(msg) {
this.aborted = true;
this.req.abort();
this.emit('fatalError', getText(msg, this.opts.url));
}
Remove _abort
and use _fatalError
everythere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@inikulin thanks, good idea
❌ Tests for the commit 9202cba have failed. See details: |
9202cba
to
44ae145
Compare
❌ Tests for the commit 44ae145 have failed. See details: |
@testcafe-build-bot \retest |
❌ Tests for the commit 44ae145 have failed. See details: |
@testcafe-build-bot \retest |
44ae145
to
47aa1a4
Compare
✅ Tests for the commit 47aa1a4 have passed. See details: |
FPR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
ping @inikulin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
/cc @churkin @miherlosev