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

POST /percy/snapshot API - Error during asset discovery not reported #1649

Open
jonleighton opened this issue Jul 4, 2024 · 8 comments
Open
Labels
✨ enhancement New feature or request

Comments

@jonleighton
Copy link

The problem

We are uploading snapshots via a manual POST /percy/snapshot API request, within a percy exec invocation.

Sometimes, there is an error during asset discovery, and we see the following log lines:

2024-07-04 10:04:12 AEST	[percy] asset-discovery - [snapshot name] - 30.013s
2024-07-04 10:04:12 AEST	[percy] Encountered an error taking snapshot: [snapshot name]
2024-07-04 10:04:12 AEST	[percy] Error: Navigation failed: Timed out waiting for the page load event

However, the POST /percy/snapshot request returns a 200 OK response, with {"success": true} in the body. Therefore our build succeeds, even though the snapshot failed, and there is no way for us to detect the problem.

I would expect a 500 Internal Server Error response in this situation, and possibly some details in the response body.

Environment

  • @percy/cli version: 1.28.8
@JackHowa
Copy link

JackHowa commented Jul 9, 2024

Possibly seeing a similar issue:

[percy:core:queue] Clearing discovery queue, queued state: 0, pending state: 0 (15ms)
[percy:core:browser] Closing browser (1ms)
[percy:core:browser] Browser closed (18ms)
[percy:core:queue] Clearing snapshot queue, queued state: 0, pending state: 0 (1ms)
[percy:core] Build not created (0ms)
[percy:cli] Error: Protocol error (Runtime.callFunctionOn): Execution context was destroyed.

  • "@percy/cli": "^1.28.8",
  • "storybook": "^8.1.10",

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 Jul 23, 2024
@JackHowa
Copy link

No, still having issues

@jonleighton
Copy link
Author

This is still an issue

@github-actions github-actions bot removed the 🍞 stale Closed due to inactivity label Jul 30, 2024
@ninadbstack
Copy link
Contributor

The POST /percy/snapshot in most SDKs work async - so it instantly returns 200. If you want to only wait for asset discovery state there is currently no provision. But if you want to wait to check the result - you can use use sync flag from sdk to get the response.

@jonleighton
Copy link
Author

@ninadbstack thanks, yes, I want to wait for asset discovery. the sync flag (from #1490) appears to block on Percy actually capturing the uploaded screenshots, which is not what we want. we want to perform asset discovery, and upload the DOM/assets, then receive a response.

@ninadbstack
Copy link
Contributor

@jonleighton Got it! Apologies for the delay, but if you need this feature please raise a support ticket with Browserstack for the same. As building it will take some time it will need to be prioritized on product roadmap.

@ninadbstack ninadbstack added the ✨ enhancement New feature or request label Aug 20, 2024
@JackHowa
Copy link

Gotcha thanks for the context @ninadbstack. This feels like a bug.

If issues aren't the best place to go for problems, can you put info on the Repo for best practices on reporting bugs? Thanks so much for reviewing the posts and maintaining this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants