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

Can't retry when taking snapshot fails #1754

Closed
bd-halkoaho opened this issue Oct 11, 2024 · 6 comments
Closed

Can't retry when taking snapshot fails #1754

bd-halkoaho opened this issue Oct 11, 2024 · 6 comments
Labels
🍞 stale Closed due to inactivity

Comments

@bd-halkoaho
Copy link

The problem

When snapshotting fails for whatever reason with message "Could not take DOM snapshot" printed from Percy, there doesn't seem to be any way to retry taking the snapshot.

Environment

  • Node version: v20.13.1
  • "@percy/cli": "^1.24.2"
  • Version of Percy SDK you’re using: "@percy/testcafe": "^1.0.3"
  • If needed, a build or snapshot ID:
  • OS version:
  • Type of shell command-line [interface]:

Details

We have EPIPE errors now and then, and haven't been able to get rid of them, so we tried to implement our own retry logic. But there is no error thrown from the snapshot function and no return value.

The place where the error is printed from is here: https://github.com/percy/percy-testcafe/blob/2b3dd7a2f2fd0f4020c3ca67b3cc1272925520a8/index.js#L57 . As you can see, the error is caught and only printed to the log, but no information provided to the caller.

It would be nice to either have the retry functionality built in to Percy, or at least give some information to the caller when snapshotting fails. I'm aware of the retry setting in Percy configuration https://www.browserstack.com/docs/percy/take-percy-snapshots/overview, but it only applies to asset discover, and doesn't seem to do anything in my case.

I'm also aware of the documentation here: https://www.browserstack.com/docs/percy/troubleshoot/self-debug#smart-debugging-snapshots-missing-or-failed mentioning reducing concurrency could fix missing snapshot issues, and I'm giving it a try. But it would still be nice to be able to retry the snapshotting for all kinds of issues. Or just know in code that it failed.

Debug logs

14:17:26 [1] [percy:client] Uploading resources for 36796423... (297ms)
14:17:26 [1] [percy:client] Uploading 1.6MB resource: ... (3ms)
14:17:26 [1] [percy:client] Uploading 13.1kB resource: /percy.1728472645915.log... (0ms)
14:17:27 [1] [percy:client] Finalizing snapshot 2008921033... (975ms)
14:17:31 [1] [percy:testcafe] Could not take DOM snapshot ""
14:17:31 [1] [percy:testcafe] Error: write EPIPE
14:17:31 [1] at WriteWrap.onWriteComplete [as oncomplete] (node:internal/stream_base_commons:95:16)
14:17:31 [1] at WriteWrap.callbackTrampoline (node:internal/async_hooks:130:17)

@ninadbstack
Copy link
Contributor

@snps-halkoaho Thanks for the detailed report. The usual reason for a DOM snapshot to fail is mostly around conflicting js/js error that causes percy's DOM snapshot js script to fail execution. EPIPE might be related to conflicting JS or most likely related to some http request failure [ i.e. testcafe proxy failure or so ]

So most likely we will need to debug root cause here to determine whats failing.

That said we can certainly add an option that would throw exception instead of suppressing it. The original goal was to make sure that a snapshot failure should not break functional tests by throwing exception.

For next steps:

  • Are you able to reproduce above with a small example ?
  • Can you move to latest version of CLI ? it has better logging which might help debugging the issue.

Let me know and we can take this forward.

@bd-halkoaho
Copy link
Author

@ninadbstack Thank you for the quick response!

Are you able to reproduce above with a small example

I'm not able to reproduce the error in any consistent manner. It happens for 1 snapshot (different one every time) in maybe 1 out of 4 percy runs. Each run has about 170 snapshots, so by quick math it happens about once in 680 snapshots. I wasn't really looking for a solution to the EPIPE error, just a way to get around it. Since it happens so rarely, I'm quite sure a retry would work. I also wouldn't discount random network glitches as the reason.

Can you move to latest version of CLI ? it has better logging which might help debugging the issue.

We are keeping up with updates, but I don't know if I'll have time to update it in the coming weeks. If I found some more information when we do update, I'll definitely keep you posted.

The original goal was to make sure that a snapshot failure should not break functional tests by throwing exception.

Not breaking the tests when snapshotting fails would definitely be a good goal. Returning something instead of throwing an error could be a less disruptive option. For example, returning a boolean that tells the caller whether the snapshotting succeeded or not.

Copy link

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.

@github-actions github-actions bot added the 🍞 stale Closed due to inactivity label Oct 29, 2024
Copy link

This issue was closed because it has been stalled for 28 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 19, 2024
@prklm10
Copy link
Contributor

prklm10 commented Nov 22, 2024

@bd-halkoaho sorry for late reply. You can use PERCY_EXIT_WITH_ZERO_ON_ERROR env variable and set it to true so if any error occurs in percy it does not throw error.
We don't have a support for returning boolean that tells the caller whether snapshot is successful or not.

  • We will take it as a FR.

@bd-halkoaho
Copy link
Author

Good news! After updating percy to 1.30.4, I haven't seen any missing snapshots anymore. So the problem I was looking for a workaround for seems to have been resolved. Thank you for the help and the great tool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍞 stale Closed due to inactivity
Projects
None yet
Development

No branches or pull requests

3 participants