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

fix Tests sometimes (frequently) lose connection (close #1647, related to DevExpress/testcafe#2711) #1729

Merged
merged 2 commits into from
Aug 17, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/request-pipeline/connection-reset-guard.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import domain from 'domain';
import os from 'os';

const connectionResetDomain = domain.create();

connectionResetDomain.on('error', err => {
if (err.code !== 'ECONNRESET') {
connectionResetDomain.removeAllListeners('error');
throw new Error(err);
}
if (err.code === 'ECONNRESET' || os.type() === 'Darwin' && err.code === 'EPIPE')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment this line

return;

connectionResetDomain.removeAllListeners('error');
throw err;
});

export default function (fn) {
Expand Down
2 changes: 2 additions & 0 deletions src/request-pipeline/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ export default class RequestPipelineContext {
this.requestFilterRules = [];
this.onResponseEventDataWithoutBody = [];

this.isBrowserConnectionReset = false;

this.reqOpts = null;
}

Expand Down
38 changes: 31 additions & 7 deletions src/request-pipeline/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,25 @@ const EVENT_SOURCE_REQUEST_TIMEOUT = 60 * 60 * 1000;

// Stages
const stages = {
0: async function fetchProxyRequestBody (ctx, next) {
0: function handleSocketError (ctx, next) {
// NOTE: In some case on MacOS, browser reset connection with server and we need to catch this exception.
if (ctx.isWebSocket) {
ctx.res.on('error', e => {
if (e.code === 'ECONNRESET' && !ctx.mock) {
if (ctx.destRes)
ctx.destRes.destroy();
else
ctx.isBrowserConnectionReset = true;
}
else
throw e;
});
}

next();
},

1: async function fetchProxyRequestBody (ctx, next) {
if (ctx.isPage && !ctx.isIframe && !ctx.isHtmlImport)
ctx.session.onPageRequest(ctx);

Expand All @@ -32,7 +50,7 @@ const stages = {
next();
},

1: function sendDestinationRequest (ctx, next) {
2: function sendDestinationRequest (ctx, next) {
if (ctx.isSpecialPage) {
ctx.destRes = createSpecialPageResponse();
next();
Expand All @@ -59,7 +77,7 @@ const stages = {
}
},

2: function checkSameOriginPolicyCompliance (ctx, next) {
3: function checkSameOriginPolicyCompliance (ctx, next) {
ctx.buildContentInfo();

if (!ctx.isKeepSameOriginPolicy()) {
Expand All @@ -77,7 +95,7 @@ const stages = {
next();
},

3: function decideOnProcessingStrategy (ctx, next) {
4: function decideOnProcessingStrategy (ctx, next) {
if (ctx.contentInfo.requireProcessing && ctx.destRes.statusCode === 204)
ctx.destRes.statusCode = 200;

Expand Down Expand Up @@ -135,7 +153,7 @@ const stages = {
next();
},

4: async function fetchContent (ctx, next) {
5: async function fetchContent (ctx, next) {
ctx.destResBody = await fetchBody(ctx.destRes);

if (ctx.requestFilterRules.length)
Expand All @@ -152,7 +170,7 @@ const stages = {
next();
},

5: async function processContent (ctx, next) {
6: async function processContent (ctx, next) {
try {
ctx.destResBody = await processResource(ctx);
next();
Expand All @@ -162,7 +180,7 @@ const stages = {
}
},

6: function sendProxyResponse (ctx) {
7: function sendProxyResponse (ctx) {
sendResponseHeaders(ctx);

connectionResetGuard(() => {
Expand Down Expand Up @@ -302,6 +320,12 @@ function sendRequest (ctx, next) {
const req = ctx.isFileProtocol ? new FileRequest(ctx.reqOpts) : new DestinationRequest(ctx.reqOpts);

req.on('response', res => {
if (ctx.isBrowserConnectionReset) {
res.destroy();

return;
}

ctx.destRes = res;
next();
});
Expand Down