kaminskypavel
commented
3 years ago as mentioned , here's what did the trick for me https://github.com/percy/percy-cypress/issues/367#issuecomment-869990300
Closed Robdel12 closed 3 years ago
kaminskypavel
commented
3 years ago as mentioned , here's what did the trick for me https://github.com/percy/percy-cypress/issues/367#issuecomment-869990300
Robdel12
commented
3 years ago Is it possible to see a video run of a failing test like this from Cypress? It's very strange it takes up to 10 seconds to POST a DOM snapshot to the local Percy server (you can email it to me fwiw: robert at percy.io)
aleksandrlat
commented
3 years ago @Robdel12 I sent you video. The list of failing tests is not exactly the same each time. Sometimes different tests failing.
I'm trying to increase default timeout like @kaminskypavel suggested.
aleksandrlat
commented
3 years ago Timeout increase worked for me too. But what can be root cause? Does percy waits for something to post snapshot?
If I understand correctly it fails on this call https://github.com/percy/percy-cypress/blob/c6c3992310ee908fb62c6075b397ec24f0b60e5a/index.js#L59
Because cypress says
Your callback function returned a promise that never resolvedI.e. callback returned. This means let domSnapshot = window.PercyDOM.serialize({ ...options, dom }); was executed successfully.
tgirgin23
commented
3 years ago That fixes it on our end as well. Hopefully an actual fix can be proposed as this is only viable temporarily.
Robdel12
commented
3 years ago Does percy waits for something to post snapshot?
We do wait for asset discovery to finish on that snapshot— that’s always been true since the inception of the SDK though. I received the video, but sadly it doesn’t clear anything up (leaves me with more questions, so not all bad!) Interesting that it looks to fail “fast”.
Has the network idle timeout been adjusted by any chance? I’ll try to comb the logs to see if asset discovery is taking a long time for a specific asset or something like that.
Robdel12
commented
3 years ago Good news: we have a good idea what's happening and an idea for a fix.
The problem: external requests are hanging asset discovery up, long enough to timeout the snapshot command. I saw a few requests taking 2-5 seconds to resolve on their own, which is more than enough hit this command timeout. These requests also were never going to be captured by asset discovery.
The fix: not track those requests in asset discovery so the CLI isn't waiting for them to resolve (incoming tomorrow -- we're at the end of our work day here now).
aleksandrlat
commented
3 years ago Has the network idle timeout been adjusted by any chance?
No I don't think so in my case
nicobatalla
commented
3 years ago Some snapshots always fail, doesn't matter if I increase the defaultCommandTimeout in Cypress or not.
describe('with session', () => {
beforeEach(() => {
cy.login();
});
it('Dashboard Snapshots', () => {
cy.visit('/');
cy.percySnapshot('Dashboard');
cy.findByTestId('toggle-sidenav').click();
cy.percySnapshot('Hidden Sidenav');
});
it('Space Snapshots', () => {
cy.visit('/spaces');
cy.findByTestId('table-container').should('be.visible');
cy.percySnapshot('Spaces');
});
// More snapshots tests
});
});In the above example, everything after "Dashboard Snapshots" (I removed all other tests so it was a shorter example) fails after the timeout. I increased the defaultCommandTimeout up to 30 seconds and it keeps throwing the same error.
aleksandrlat
commented
3 years ago Yeah one of my tests failed today with 10000 timeout too
tomkaos
commented
3 years ago I have the same... Should we need to downgrade to a version?
Robdel12
commented
3 years ago This should be taken care of once this PR ships today: https://github.com/percy/cli/pull/400
Robdel12
commented
3 years ago v1.0.0-beta.58 has been released and should take care of this. The builds I debugged all had requests in asset discovery taking a while to respond (external requests that the SDK wasn't going to capture, but was still waited for them to resolve before resolving the percySnapshot promise)
nicobatalla
commented
3 years ago @Robdel12 v1.0.0-beta.58 fixed the issue on my end, thank you
aleksandrlat
commented
3 years ago I got the error again :(
aleksandrlat
commented
3 years ago 5 tests failed with this error again nightly. We upgraded to "v1.0.0-beta.58" immediately when it was released.
Robdel12
commented
3 years ago Can you post logs (--verbose) from the test run? It means there's requests we're waiting to capture that take a while to respond. Verbose logs would show which ones are taking long. Also would be helpful to know what the network idle timeout is set to (anything higher than 500 is too high -- that's 500ms of idle to wait for for 0 requests to be made before stopping asset discovery)
oeddyo
commented
3 years ago Hi @Robdel12 we are seeing all our percy snapshots failing from today. We use retry in cypress but in the past percy was running fine (so I suppose there's no retry for percy commands). However starting today it becomes super flaky. Could you take a look?
oeddyo
commented
3 years ago Tried to increase timeout also worked for me. It seems like a hack though
Robdel12
commented
3 years ago Sharing the full verbose logs from the test run would be the only way to debug this -- the issue is there's assets take longer than the set timeout to resolve (so it could be the apps test server take a while to respond with the assets). The verbose logs will show which assets are taking a while and then we can figure out what's next from thre
oeddyo
commented
3 years ago I don't know if you have any client side logging/viz.. recent 3 days it's becoming recurring. I believe it should be something all your users experience
How do you enable verbose in percy/cypress? I can help add one. @Robdel12
Robdel12
commented
3 years ago Anything with the SDKs will be client side. You can pass --verbose to the CLI and then post the logs here, then we'll be able to take a look
aleksandrlat
commented
3 years ago @Robdel12 is this possible to catch this error on Percy level and re-throw with better error message explaining what caused the issue? If you can log it you should probably do this, right?
kamal
commented
3 years ago I'm seeing an issue where a page that has Stripe Elements on it will hang indefinitely. Currently running @percy/cli 1.0.0-beta.60.
When running percy/cli in verbose mode, I don't even see the debug logs output for this particular page.
I then compared the output against a run against Percy 2.x. I see Cypress resolving an asset from Stripe against the wrong base URL. For example, I see
GET /billing/fingerprinted/css/ui-shared-bbb176702b532fdcf3153c8a7f0d754f.css 404 3.742 ms - 211On Percy 2, it'll just ignore this and move on.
That path does not exist on my local filesystem. The confusion seems to stem from it not taking into account the asset is being loaded inside the Stripe iframe.
In the Stripe iframe, you'll see the path to the CSS defined as
<link href="fingerprinted/css/ui-shared-bbb176702b532fdcf3153c8a7f0d754f.css" rel="stylesheet">And the file should rightly be loaded from https://js.stripe.com/v3/fingerprinted/css/ui-shared-bbb176702b532fdcf3153c8a7f0d754f.css
I believe Percy is stuck on waiting to fetch this file which Cypress wrongly reports the path as local.
Robdel12
commented
3 years ago Hey @kamal! Would you be able to share the full verbose SDK logs from a run where it hangs? We fixed the issue that agent had (which carried over to CLI) in this PR: https://github.com/percy/cli/pull/405
I'd like to see the full logs to see what's going on in asset discovery and which asset it is exactly that's causing tests to hang.
kamal
commented
3 years ago @Robdel12 pasting it below. the problematic test is the 2nd one to run. there is no output from percy at all. It just hangs there until you cancel the workflow.
Compare this run against @percy/cypress@2 and you'll see the 404 for the Stripe asset, but with base url confusion
Robdel12
commented
3 years ago 🤨 Hm, I don't see any requests in Percy's logs for /billing/fingerprinted/css/.... -- it doesn't look like that asset is hanging asset discovery. This isn't a Percy log (it's Cypress). If it's being discovered by asset discovery, then it should log out for that snapshot. I also don't see this log in the CLI logs, either (but it is in the agent logs)
GET /billing/fingerprinted/css/ui-shared-bbb176702b532fdcf3153c8a7f0d754f.css 404 1.518 ms - 211Are these the full logs (unedited)? Any ideas for what caused this error?
Error: The operation was canceled.Was it CI that canceled the run? I don't see any errors/exceptions in the logs to explain that. The second test (Running: billing.subscribe.spec.js (2 of 20)) doesn't capture a Percy snapshot before the exception is hit, either.
I also noticed enableJavaScript: true is set, any reasons for that? This could cause issues too (since JS would execute in asset discovery & then in our browsers). We generally highly recommend having JS disabled. Apps are almost never prepared to rerun JS over fully formed DOM.
kamal
commented
3 years ago it's unedited. the error is caused by me cancelling the github action after letting it sit there for 20 minutes doing nothing. enableJavascript is usually false, but i was trying various things to see if it would fix the issue. I can revert this back to false and it'll hit the same issue in the exact same spot.
if i comment out the cy.percySnapshot() call in my cypress script, the test will finish.
Robdel12
commented
3 years ago I think this is related to https://github.com/percy/cli/pull/453 -- can you give @percy/cli@1.0.0-beta.61 a shot? There were a few similar issues on other SDKs that were solved by that bugfix
kamal
commented
3 years ago @Robdel12 no luck, still same results
Robdel12
commented
3 years ago Hm, it doesn't seem like this is related to the issue we're commenting on. Can you open an new issue with verbose logs from beta 61 (I see build 11834913 failed with a finalize error)? If possible a reproduction for us to play with would be awesome (it's hard to debug env issues without it)
github-actions[bot]
commented
3 years ago This issue is stale because it has been open for more than 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.
Robdel12
commented
3 years ago 🤦🏻♂️I knew I forgot an issue to update. Can you give beta 65 a try? I think https://github.com/percy/cli/pull/490 fixed the issue you were seeing
kamal
commented
3 years ago @Robdel12 i just tried this with beta 65 and it did not fix the issue unfortunately.
kamal
commented
3 years ago @Robdel12 i no longer have this issue with beta 66!
wwilsman
commented
3 years ago Going to close this now that it seems to be resolved. 👍
Edit: this will be fixed by an upstream CLI PR: https://github.com/percy/cli/pull/400
Picking up from #367 (unrelated parallel issues going on)
cc: @kaminskypavel / @aleksandrlat
Does this happen for a specific snapshot each time? Or is it a different snapshot? If it's the same snapshot, that's good since it's reproducible/repeatable and likely something to do with the DOM.