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

Percy server fails to respond with Cypress open #315

Closed
jonpitch opened this issue Apr 9, 2021 · 11 comments · Fixed by #321
Closed

Percy server fails to respond with Cypress open #315

jonpitch opened this issue Apr 9, 2021 · 11 comments · Fixed by #321

Comments

@jonpitch
Copy link

jonpitch commented Apr 9, 2021

i have a new gatsby project, setup with cypress and percy-cypress, where i'm experiencing some crashing that i'm trying to narrow down. you can find a reproduction repository here.

to reproduce:

  • start the gatsby site: npm run develop
  • in a new tab:
    • ./node_modules/.bin/cypress run (this works)
    • ./node_modules/.bin/cypress open -> run tests -> crash

specifically disabling percy doesn't seem to work either:

  • start the gatsby site: npm run develop
  • in a new tab:
    • PERCY_ENABLE=false ./node_modules/.bin/cypress open -> run tests -> crash

my initial thought was the percy server must be running no matter what, however if you then:

  • start the gatsby site: npm run develop
  • in a new tab:
    • percy exec -- ./node_modules/.bin/cypress open -> run tests
    • i see as output:
[percy] Skipping visual tests - Missing Percy token
[percy] Running "./node_modules/.bin/cypress open"

i still experience a crash. additionally, supplying a token doesn't help either. while i don't experience a crash in this particular scenario, i don't my my team uploading snapshots running tests locally. i haven't yet been able to disable percy for local development, with PERCY_BRANCH=local or PERCY_ENABLE=false. and fwiw, i'm using a Mac, on BigSur, version 11.2.3.

let me know if you need more information. thanks!

@Robdel12 Robdel12 transferred this issue from percy/percy-cypress Apr 9, 2021
@Robdel12 Robdel12 changed the title gatsby cy.percySnapshot crash @percy/dom serialization issue with cypress open/gatsby Apr 9, 2021
@Robdel12 Robdel12 transferred this issue from percy/cli Apr 9, 2021
@Robdel12 Robdel12 changed the title @percy/dom serialization issue with cypress open/gatsby Percy server fails to respond with Cypress open Apr 9, 2021
@Robdel12
Copy link
Contributor

Robdel12 commented Apr 9, 2021

Prematurely transferred based on a glance. Whoops! Error stack trace that's helpful:

cy.then() timed out after waiting 4000ms.

Your callback function returned a promise that never resolved.

The callback function was:

async () => {
    // Check if Percy is enabled
    if (!await utils.isPercyEnabled()) {
      return cylog('Not running', { name });
    }

    // Inject @percy/dom
    if (!window.PercyDOM) {
      // eslint-disable-next-line no-eval
      eval(await utils.fetchPercyDOM());
    }

    // Serialize and capture the DOM
    return cy.document({ log: false }).then(dom => {
      let domSnapshot = window.PercyDOM.serialize({ ...options, dom });

      // Post the DOM snapshot to Percy
      return utils.postSnapshot({
        ...options,
        environmentInfo: ENV_INFO,
        clientInfo: CLIENT_INFO,
        domSnapshot,
        url: dom.URL,
        name
      }).then(() => {
        // Log the snapshot name on success
        cylog(name, { name });
      }).catch(error => {
        // Handle errors
        log.error(Could not take DOM snapshot "${name}");
        log.error(error);
      });
    });
  }Learn more

@Robdel12
Copy link
Contributor

Robdel12 commented Apr 9, 2021

@wwilsman Let's implement what Cypress does (and what we talked about) where we just completely disable Percy if anyone uses cypress open (but also good to know what's causing this to hang in interactive mode?)

@wwilsman
Copy link
Contributor

wwilsman commented Apr 9, 2021

@Robdel12 What cypress does is disable the snapshot method when in interactive mode. If we implemented it in the same way, it would cause empty builds to be sent to Percy — something we probably don't want. Properly disabling Percy from within an SDK is not possible currently (but will hopefully be possible soon with planned internal core queue changes).

As to the actual issue, it appears as though the request to the server is taking longer than 4s to fail. It would be helpful to have debug logs using percy exec --verbose -- ... or PERCY_LOGLEVEL=debug to see why exactly the server is taking so long to process the request.

It's possible we simply need to tone down the request timeout since we are using an internal Cypress API without managing timeouts ourselves. But it's still interesting that these requests are timing out even when Percy is not running.

@wwilsman
Copy link
Contributor

wwilsman commented Apr 9, 2021

After quite a bit of debugging, I've arrived at the conclusion that this isn't actually a Percy issue, but unfortunately I still don't know where the issue belongs.

You can replace cy.percySnapshot() with cy.request('http://unreachable-url:1337/') and it too will hang and crash.

I'll continue to try and help debug for a little longer, but since this is no longer an issue with Percy, I can't dedicate too much of my work day to it.

Closing this unless somebody can report that this is indeed somehow Percy related.

@wwilsman wwilsman closed this as completed Apr 9, 2021
@jonpitch
Copy link
Author

jonpitch commented Apr 9, 2021

@wwilsman thanks for all the time you spent here. is my path forward to just not use Percy until i can figure out the upstream problem?

@wwilsman
Copy link
Contributor

wwilsman commented Apr 9, 2021

@jonpitch Still haven't located the issue, but I did make some progress and find a workaround for you.

I tested this scenario in a fresh Cypress project and didn't seem to have any issues. There was no crash and the correct error was logged. If I had to guess, I would say this is probably a dependency resolution issue.

However, I did find this open issue on the Cypress repo: cypress-io/cypress#15101

I confirmed that downgrading your reproduction to Cypress 6.4.0 works as intended.

Going to open this back up until that issue is resolved upstream for others that run into this.

@wwilsman wwilsman reopened this Apr 9, 2021
@jonpitch
Copy link
Author

jonpitch commented Apr 9, 2021

thanks @wwilsman! confirmed here that 6.4.0 works also. apologies, i can't believe i missed that open issue over at cypress-io.

@MarosPistej
Copy link

MarosPistej commented Apr 13, 2021

to my issue #320 which is most likely same as this one worked also
modifying cypress/support/index.js file and include code bellow

import { isPercyEnabled } from "@percy/sdk-utils";
isPercyEnabled();

this works even with Cypress 7.1.0

in fact calling isPercyEnabled() anytime but before invoking cy.percySnapshot() work..

@cassus
Copy link

cassus commented Apr 13, 2021

Thanks @MarosPistej for the workaround, it worked for me as well!

This was my crash error message, I put it here, so others may find the workaround easier

RangeError: Maximum call stack size exceeded
    at _deconstructPacket (<...>node_modules/socket.io-parser/dist/binary.js:21:28)
    at _deconstructPacket (<...>node_modules/socket.io-parser/dist/binary.js:40:32)

    <15k lines of the same message>

    at _deconstructPacket (<...>node_modules/socket.io-parser/dist/binary.js:32:26)
    at Object.deconstructPacket (<...>node_modules/socket.io-parser/dist/binary.js:16:17)
    at Encoder.encodeAsBinary (<...>node_modules/socket.io-parser/dist/index.js:81:41)
    at Encoder.encode (<...>node_modules/socket.io-parser/dist/index.js:43:29)
    at Client._packet (<...>node_modules/socket.io/dist/client.js:167:44)
    at Socket.packet (<...>node_modules/socket.io/dist/socket.js:161:21)
    at <...>node_modules/socket.io/dist/socket.js:270:18
    at <...>/7.1.0/Cypress.app/Contents/Resources/app/packages/server/lib/socket-base.js:325:28
    at tryCatcher (<...>/7.1.0/Cypress.app/Contents/Resources/app/packages/server/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (<...>/7.1.0/Cypress.app/Contents/Resources/app/packages/server/node_modules/bluebird/js/release/promise.js:547:31)
    at Promise._settlePromise (<...>/7.1.0/Cypress.app/Contents/Resources/app/packages/server/node_modules/bluebird/js/release/promise.js:604:18)
    at Promise._settlePromise0 (<...>/7.1.0/Cypress.app/Contents/Resources/app/packages/server/node_modules/bluebird/js/release/promise.js:649:10)
    at Promise._settlePromises (<...>/7.1.0/Cypress.app/Contents/Resources/app/packages/server/node_modules/bluebird/js/release/promise.js:725:18)
    at _drainQueueStep (<...>/7.1.0/Cypress.app/Contents/Resources/app/packages/server/node_modules/bluebird/js/release/async.js:93:12)
    at _drainQueue (<...>/7.1.0/Cypress.app/Contents/Resources/app/packages/server/node_modules/bluebird/js/release/async.js:86:9)
    at Async._drainQueues (<...>/7.1.0/Cypress.app/Contents/Resources/app/packages/server/node_modules/bluebird/js/release/async.js:102:5)
    at Immediate.Async.drainQueues [as _onImmediate] (<...>/7.1.0/Cypress.app/Contents/Resources/app/packages/server/node_modules/bluebird/js/release/async.js:15:14)
    at processImmediate (internal/timers.js:461:21)

@Robdel12
Copy link
Contributor

Robdel12 commented Apr 14, 2021

👋🏼 We shipped an update (v3.1.0) that side steps the bug. Hopefully it's fixed upstream soon, but we should no longer do anything in interactive mode.

@cassus
Copy link

cassus commented Apr 15, 2021

Even using v3.1.0 I have the same crash as before if I remove the isPercyEnabled() call from commands.ts.

Calling isPercyEnabled() in commands.ts still avoids the crash.

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 a pull request may close this issue.

5 participants