percy / percy-cypress

Visual testing with Cypress and Percy
https://percy.io
MIT License
340 stars 41 forks source link

Percy server fails to respond with Cypress open #315

Closed jonpitch closed 3 years ago

jonpitch commented 3 years ago

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:

specifically disabling percy doesn't seem to work either:

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

let me know if you need more information. thanks!

Robdel12 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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.

jonpitch commented 3 years ago

@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 commented 3 years ago

@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: https://github.com/cypress-io/cypress/issues/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.

jonpitch commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

👋🏼 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 commented 3 years ago

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.