cypress-io / cypress

Fast, easy and reliable testing for anything that runs in a browser.
https://cypress.io
MIT License
46.99k stars 3.18k forks source link

Intercept with req.continue throws "Error: Socket closed before finished writing response" #26248

Open mirobo opened 1 year ago

mirobo commented 1 year ago

Current behavior

Application behaviour

I have an Angular app which triggers multiple requests on the same endpoint:

Main issue

See test req.continue() / fails with Error "Socket closed before finished writing response"

In the cancelled request, we see the error CypressError: A callback was provided to intercept the upstream response, but a network error occurred while making the request: Error: Socket closed before finished writing response:

image

This error results in a failed test, if we call cy.wait('@myAlias') or even cy.wait(2000) (!)

If we don't call cy.wait(...) then the error will be ignored and will only be visible in the console. To reproduce this add .only to the test "does not fail with "Error Socket closed before finished writing response", but should fail" and maybe run multiple times. A red, cancelled request should appear and the error is visible in the console when clicking on the red, cancelled request.

The same issue happens "consistentlyer ;-)" when using req.on('response') instead of req.continue(..) and error is hidden inside the console:

image

In my setup I cannot use req.on('response') because it triggers other errors and issues (I think I saw it somehow mess up the order of interceptions in the command log?). After all req.continue() should just work fine.

Desired behavior

There is a benefit in this kind of failing test, so that we know that we should/could replace switchMap with exhaustMap to prevent multiple service calls. But this is not the main reason I created this ticket. Previously in Cypress 10.7.0 these tests would not fail and the error was probably just hidden.

I expect that the error gets either propagated in every situation and that it can be handled or ignored explicitly or that the error is ignored completely.

I see that using req.on('response') would be more sense since this is called only before an actual response is sent to the browser. But in this scenario, both should just work fine. If the request was cancelled, Cypress shouldn't care about sending a response anyway and it shouldn't do anything in req.continue(..).

Test code to reproduce

https://github.com/mirobo/cypress-cloud-module-api

Cypress Version

12.8.1

Node version

v18.13.0

Operating System

Windows 10

Debug Logs

No response

Other

No response

mirobo commented 1 year ago

Hey @astone123 :) Could you already look into this issue? Do you need some more information, logs, details? Currently this is sort of a blocker for us because we cannot update to Cypress 12+ this way and going for an older 12.x version will probably trigger other errors which have already been resolved. I'd like to stay up to date :)

astone123 commented 1 year ago

@mirobo thanks for the detailed issue and reproduction. It looks like we shouldn't be surfacing this error and failing the test. Routing this to our e2e team

mirobo commented 1 year ago

Could be related or even have been introduced by #18544 / https://github.com/cypress-io/cypress/pull/24709 ?

Matthew-Turner-ptml commented 1 year ago

Is there a work around to ignore these errors and stop it failing tests?

mirobo commented 1 year ago

I tought of writing code to temporarly just ignore this error by patching

C:.....\Cypress\Cache\12.10.0\Cypress\resources\app\node_modules\bluebird\js\release\util.js

..and as stupid as it sounds: Replacing

function tryCatcher() {
    try {
        var target = tryCatchTarget;
        tryCatchTarget = null;
        return target.apply(this, arguments);
    } catch (e) {
        errorObj.e = e;
        return errorObj;
    }
}

with

function tryCatcher() {
    try {
        var target = tryCatchTarget;
        tryCatchTarget = null;
        return target.apply(this, arguments);
    } catch (e) {
        errorObj.e = e;
        if(!`${e}`.includes('Socket closed before finished writing response')) {
            return errorObj;
        }
        // return errorObj;
    }
}

I'm hoping this bug gets fixed with the next release so I don't have to go with this ugly hack, of which I don't even know it does what I expect xD. But be my guest ;-)

mirobo commented 1 year ago

@mjhenkes Would it be possible for you to check whether my workaround is viable? I'm not sure what side effects there are and I'd potentially miss important errors with "socket closed before finished writing response".

Patching Cypress the easy way with https://docs.cypress.io/guides/references/troubleshooting#Patch-Cypress is also not possible because the code in this example is not inside node_modules but in the Cypress binary folder.

Or is the solution simple by just removing request.res.off('close', onClose) here: https://github.com/cypress-io/cypress/blob/develop/packages/net-stubbing/lib/server/middleware/request.ts#L115 ?

Or just never throw this error at all here? https://github.com/cypress-io/cypress/blob/develop/packages/proxy/lib/http/index.ts#L154

Some guidance here is very much appreciated 😌

mirobo commented 1 year ago

@astone123 @mjhenkes I was trying the above workaround, but it just throw another error. So I tried to patch 12.10.0/Cypress/resources/app/node_modules/@packages/proxy/lib/http/index.js by replacing

_onError(new Error('Socket closed before finished writing response.'));

with

//_onError(new Error('Socket closed before finished writing response.'));

and

return onError(new Error('Socket closed before finished writing response'));

with

return; // onError(new Error('Socket closed before finished writing response'));

But I'm unable to patch that file because it simply does not exist in the Linux ZIP file (installed via npm.. I also inspected the ZIP file from https://download.cypress.io/desktop/12.10.0?platform=linux&arch=x64 )

The folders in the Linux binary are:

/root/.cache/Cypress/12.10.0/Cypress/resources/app/node_modules/@packages/proxy
/root/.cache/Cypress/12.10.0/Cypress/resources/app/node_modules/@packages/proxy/node_modules
/root/.cache/Cypress/12.10.0/Cypress/resources/app/node_modules/@packages/proxy/node_modules/iconv-lite
/root/.cache/Cypress/12.10.0/Cypress/resources/app/node_modules/@packages/proxy/node_modules/iconv-lite/.github

Where as on Windows, the folder proxy/lib/http exists (and also the file :-))

..\AppData\Local\Cypress\Cache\12.10.0\Cypress\resources\app\node_modules\@packages\proxy\lib
..\AppData\Local\Cypress\Cache\12.10.0\Cypress\resources\app\node_modules\@packages\proxy\node_modules
..\AppData\Local\Cypress\Cache\12.10.0\Cypress\resources\app\node_modules\@packages\proxy\lib\http
..\AppData\Local\Cypress\Cache\12.10.0\Cypress\resources\app\node_modules\@packages\proxy\lib\http\util
..\AppData\Local\Cypress\Cache\12.10.0\Cypress\resources\app\node_modules\@packages\proxy\node_modules\iconv-lite
..\AppData\Local\Cypress\Cache\12.10.0\Cypress\resources\app\node_modules\@packages\proxy\node_modules\iconv-lite\.github
..\AppData\Local\Cypress\Cache\12.10.0\Cypress\resources\app\node_modules\@packages\proxy\node_modules\iconv-lite\encodings
..\AppData\Local\Cypress\Cache\12.10.0\Cypress\resources\app\node_modules\@packages\proxy\node_modules\iconv-lite\lib
..\AppData\Local\Cypress\Cache\12.10.0\Cypress\resources\app\node_modules\@packages\proxy\node_modules\iconv-lite\encodings\tables

Where is the file @packages/proxy/lib/http/index.js under Linux? Even if I search for "Socket closed" over all JS files, it shows absolutely nothing 😭.

Any piece of advice is very much appreciated since we are stuck here and it seems like it's getting out of hand with increasingly more flaky tests showing up because of this issue. I might need to create a fork because patching with https://docs.cypress.io/guides/references/troubleshooting#Patch-Cypress seems kinda irrelevant as of now with the compiled index.jsc file and the binary files cannot be patched easily OS independent.

mirobo commented 1 year ago

@Matthew-Turner-ptml

Is there a work around to ignore these errors and stop it failing tests?

I finally managed to have a workaround with ignoring the error... super ugly.. but it seems to work for now:

const path = require('path');
const fs = require('fs');

const fileToPatch = path.resolve(process.env.CYPRESS_CACHE_FOLDER, '12.10.0/Cypress/resources/app/packages/runner/dist/cypress_runner.js');
let content = fs.readFileSync(fileToPatch).toString();
content = content.replace('function tryCatcher() {\n' +
        '    try {\n' +
        '        var target = tryCatchTarget;\n' +
        '        tryCatchTarget = null;\n' +
        '        return target.apply(this, arguments);\n' +
        '    } catch (e) {\n' +
        '        errorObj.e = e;\n' +
        '        return errorObj;\n' +
        '    }\n' +
        '}', 'function tryCatcher() {\n' +
        '    try {\n' +
        '        var target = tryCatchTarget;\n' +
        '        tryCatchTarget = null;\n' +
        '        return target.apply(this, arguments);\n' +
        '    } catch (e) {\n' +
        '        errorObj.e = e;\n' +
        "\t\tif(!`${e}`.includes('Socket closed before finished writing response')) {\n" +
        '\t\t\treturn errorObj;\n' +
        '\t\t}\n' +
        '        // return errorObj;\n' +
        '    }\n' +
        '}');
fs.writeFileSync(fileToPatch, content);

You may need to adjust the base path and Cypress version within the path. I run it in only in Gitlab CI with a docker image, so I don't care about creating a backup file.

Gopala7721 commented 1 year ago

we are facing same issue after upgrading to cypress 12.13.0 please provide us the fix

Gopala7721 commented 1 year ago

we are facing same issue after upgrading to cypress 12.13.0 please provide us the fix

mirobo commented 1 year ago

Hey @mjhenkes

Do you have some guidance in how to properly resolve this issue? My hack above works and successfully ignores the thrown error. But I'd like to know how Cypress would want this issue to be fixed so I could open a PR for this. I'm aware that this is some serious core code right there, so maybe there are other ways in which I can contribute if the Cypress team decides to fix the issue on their own?

There may also be some overlap with https://github.com/cypress-io/cypress/issues/19326. In general I think it could also be a really nice feature to be able to generally detect cancelled requests and have the ability to either ignore cancelled requests on specific routes or treat them as errors. Especially in applications that use NGRX this could be a huge benefit to prevent unwanted side effects, duplicate triggers, etc.

chwoerz commented 1 year ago

@Matthew-Turner-ptml @Gopala7721 @mirobo @mjhenkes I have solved this issue by changing the req.continue to

req.on('response', response => /* do something with the response */)

This on-trigger will only use responses sent back and will not fail on cancelled requests, because these will not trigger this on-response.

Because req.continue needs a valid response to work. https://docs.cypress.io/api/commands/intercept#Request-events

mirobo commented 1 year ago

I quote myself: In my setup I cannot use req.on('response') because it triggers other errors and issues (I think I saw it somehow mess up the order of interceptions in the command log?)

I cannot use req.on('response',..).. or I need to spend more time in the subsequent issues I had with it (and if I recall correctly I had way more issues in our tests).

Still, the error "Socket closed before finished writing response" should not be surfaced. It's ok to cancel requests in an app and cy.wait() should only wait for requests that actually happend (and not cancelled ones?)

nghoiying04 commented 1 year ago

Same issue happened on my cypress test cases, especially on CI - we run it on docker. It randomly displayed with cancel request while it works good in the local desktop.

rakeshnambiar commented 1 year ago

Hi, I have the same issue even on req.continue I am using Cypress version 12.17.3

cy.intercept(
    { url: '*' },
    (req) => {
      req.continue((res) => {
        calls.push({
StefMcW-Paper commented 1 year ago

We're seeing this same issue impact around a 1/3rd of our tests, randomly, when running with 12.17.3. We're updating from 10.x where we did not see this issue.

nghoiying04 commented 1 year ago

You may try with 12.17.4. Currently, my tests do not encounter the error randomly.

rakeshnambiar commented 1 year ago

I didn't see much improvements on 12.17.4

hiepluong2205 commented 1 year ago

Same error "Socket closed before finished writing response" while intercepting an GraphQL response. Reproducible randomly about 1/5 times. Which made the tests as flaky ones.


cy.wait(`@gql${operationName}`).then((interception: any) => {
      expect(interception.response?.body.errors).to.be.undefined;
      expect(interception.response?.body.data).not.to.be.null;
      ...
    });
mirobo commented 1 year ago

Cypress didn't fix anything regarding this issue yet. My old example is and was 100% reproducible. Patching Cypress works just fine but I haven't checked if this is still possible with Cypress 13 (if not it's a show stopper).

frankind commented 12 months ago

I am using Cypress version 13.3.2 and still found same problems, it would be good if Cypress can fix this issue. Thx in advance.

KevinFerreira commented 11 months ago

Running into a very similar error, was hoping it gets addressed with cypress soon

KevinFerreira commented 11 months ago

@Matthew-Turner-ptml @Gopala7721 @mirobo @mjhenkes I have solved this issue by changing the req.continue to

req.on('response', response => /* do something with the response */)

This on-trigger will only use responses sent back and will not fail on cancelled requests, because these will not trigger this on-response.

Because req.continue needs a valid response to work. https://docs.cypress.io/api/commands/intercept#Request-events

Small update - this actually worked for me. I had a problem where a query that gets run on every single page would almost always initially fail, but succeed immediately after in a second try, but cypress would hyperfixate on the first fail, which would make the test also fail. However, with this change, all those flaky tests pass consistently now.

Still hoping there's some internal cypress fix for this, but glad to have resolved the issue (for now...🤔)

mirobo commented 11 months ago

Sure there are more critical issues to be fair and my workaround with patching Cypress still works with 12.17.1. But still.. it's 8 months now and to me Cypress' intercept is a key feature...

HartS commented 10 months ago

@mirobo I assumed the docs around continue were incomplete. I found I had good luck with continue when I end the continue callback with res.send()

rakeshnambiar commented 9 months ago

I can see this problem more often in the latest 13.6.3 version.

A callback was provided to intercept the upstream response, but a network error occurred while making the request:
Error: Socket closed before finished writing response
    at <embedded>:4432:40137
    at tryCatcher (/root/.cache/Cypress/13.6.3/Cypress/resources/app/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/root/.cache/Cypress/13.6.3/Cypress/resources/app/node_modules/bluebird/js/release/promise.js:512:31)
    at Promise._settlePromise (/root/.cache/Cypress/13.6.3/Cypress/resources/app/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/root/.cache/Cypress/13.6.3/Cypress/resources/app/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/root/.cache/Cypress/13.6.3/Cypress/resources/app/node_modules/bluebird/js/release/promise.js:694:18)
    at _drainQueueStep (/root/.cache/Cypress/13.6.3/Cypress/resources/app/node_modules/bluebird/js/release/async.js:138:12)
    at _drainQueue (/root/.cache/Cypress/13.6.3/Cypress/resources/app/node_modules/bluebird/js/release/async.js:131:9)
    at Async._drainQueues (/root/.cache/Cypress/13.6.3/Cypress/resources/app/node_modules/bluebird/js/release/async.js:147:5)
    at Immediate._onImmediate (/root/.cache/Cypress/13.6.3/Cypress/resources/app/node_modules/bluebird/js/release/async.js:17:14)
    at process.processImmediate (node:internal/timers:476:21)
Route: {
  "url": "*"
}
mirobo commented 8 months ago

@Matthew-Turner-ptml

Is there a work around to ignore these errors and stop it failing tests?

I finally managed to have a workaround with ignoring the error... super ugly.. but it seems to work for now:

const path = require('path');
const fs = require('fs');

const fileToPatch = path.resolve(process.env.CYPRESS_CACHE_FOLDER, '12.10.0/Cypress/resources/app/packages/runner/dist/cypress_runner.js');
let content = fs.readFileSync(fileToPatch).toString();
content = content.replace('function tryCatcher() {\n' +
        '    try {\n' +
        '        var target = tryCatchTarget;\n' +
        '        tryCatchTarget = null;\n' +
        '        return target.apply(this, arguments);\n' +
        '    } catch (e) {\n' +
        '        errorObj.e = e;\n' +
        '        return errorObj;\n' +
        '    }\n' +
        '}', 'function tryCatcher() {\n' +
        '    try {\n' +
        '        var target = tryCatchTarget;\n' +
        '        tryCatchTarget = null;\n' +
        '        return target.apply(this, arguments);\n' +
        '    } catch (e) {\n' +
        '        errorObj.e = e;\n' +
        "\t\tif(!`${e}`.includes('Socket closed before finished writing response')) {\n" +
        '\t\t\treturn errorObj;\n' +
        '\t\t}\n' +
        '        // return errorObj;\n' +
        '    }\n' +
        '}');
fs.writeFileSync(fileToPatch, content);

You may need to adjust the base path and Cypress version within the path. I run it in only in Gitlab CI with a docker image, so I don't care about creating a backup file.

I'm still living with my patch and at least I'm glad it works with 13+ (currently we're on 13.6.4). Looking forward to some kind of reaction from Cypress on this one.. it's almost been a year now..

zbjornson commented 8 months ago

A workaround that appears to work (if your interceptor isn't very sensitive to the order of hooks) is to use req.on("before:response", res => { }) instead of req.continue(res => {}).

tputnoky commented 5 months ago

Is there any news on this? I'm consistently encountering the "Error Socket closed before finished writing" error in almost every test run.

My app heavily relies on updates from Firestore. Whenever this error pops up, the test fails—not due to the error itself, but because the app fails to update. So, simply ignoring the error (as suggested previously) isn't a viable option for me.

Unfortunately, this makes our end-to-end tests unusable since we're getting false negatives all the time. 😔

MadnezGirl commented 3 months ago

I'm experiencing the same problem on Cypress version 12.17.3

Error: Socket closed before finished writing response

My code in beforeEach that triggered this issue

image
mirobo commented 3 months ago

@mirobo I assumed the docs around continue were incomplete. I found I had good luck with continue when I end the continue callback with res.send()

In my real tests I already used res.send(..) within req.continue like this and it failed nonetheless:

cy.intercept({ url: myUrl, method: method }, (req) => {
  req.continue((res) => {
    res.send(someStatusCode, someOtherContent);
  });
}).as(this.alias);

A workaround that appears to work (if your interceptor isn't very sensitive to the order of hooks) is to use req.on("before:response", res => { }) instead of req.continue(res => {}).

This variant works partially in my small test project: Whenever I intercept a route twice with the same alias setup, verifying the total number of API calls is different.

With req.continue(res => {}) I can verify that the API has been called, let's say 3 times (no matter how many interceptions I set up). When I use req.on("before:response", res => { }), then the amount of API calls reported from Cypress are multiplied by how many times the interception was set up.

Due to the nature of my tests it is sometimes quite complicated to setup the interceptions, so it is easier to setup some general interceptions for several tests and then override specific interceptions within the test.

CN91 commented 1 month ago

Also happens when using intercept and stubbing the response:

 cy.intercept('GET', '/api/release-notes**', {
        fixture: 'release-notes.json'
    }).as('release-notes');
RaoAsim commented 1 month ago

Its been more than a year and we still haven't got any solution to this, considering my test cases are dynamic and rely on streaming response from BE, most of my test fail cause of this issue and I may end up switching to some other library. @jennifer-shehane @astone123

mirobo commented 1 month ago

Its been more than a year and we still haven't got any solution to this, considering my test cases are dynamic and rely on streaming response from BE, most of my test fail cause of this issue and I may end up switching to some other library. @jennifer-shehane @astone123

try my "patch" here https://github.com/cypress-io/cypress/issues/26248#issuecomment-1963632871 to suppress this specific error. so far we didn't have any issues and we didn't observe tests that should have failed.

I hope for a permanent fix in Cypress too 🤗

RaoAsim commented 1 month ago

Its been more than a year and we still haven't got any solution to this, considering my test cases are dynamic and rely on streaming response from BE, most of my test fail cause of this issue and I may end up switching to some other library. @jennifer-shehane @astone123

try my "patch" here #26248 (comment) to suppress this specific error. so far we didn't have any issues and we didn't observe tests that should have failed.

I hope for a permanent fix in Cypress too 🤗

Can you share your github action code? and do I need to run this file after the npm i?

mirobo commented 1 month ago

try my "patch" here #26248 (comment) to suppress this specific error. so far we didn't have any issues and we didn't observe tests that should have failed. I hope for a permanent fix in Cypress too 🤗

Can you share your github action code? and do I need to run this file after the npm i?

there's no github action. in your pipeline script just run the script with node before you run cypress. also adjust the path according to the used version.

RaoAsim commented 1 month ago

try my "patch" here #26248 (comment) to suppress this specific error. so far we didn't have any issues and we didn't observe tests that should have failed. I hope for a permanent fix in Cypress too 🤗

Can you share your github action code? and do I need to run this file after the npm i?

there's no github action. in your pipeline script just run the script with node before you run cypress. also adjust the path according to the used version.

Didn't work in my case, still getting the error... you can check this for more info: https://github.com/cypress-io/cypress/issues/29938