-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
chore: add trace viewer file upload error handling #10243
chore: add trace viewer file upload error handling #10243
Conversation
Looks like #10127 can have a simpler fix. What exact scenario are we fixing here? |
370c7e0
to
c09143f
Compare
try { | ||
await traceModel.load(url, progress); | ||
} catch (error) { | ||
if (fetchFromInbuiltTraceViewerServer && error instanceof Error && error.message === 'HTTP error Not Found') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since local file URLs also get loaded over HTTP, check if its done via the internal trace viewer web-server and rewrite it that the local file could not be found (instead of showing the http error).
c09143f
to
31cb34f
Compare
// Don't re-use blob file URLs on page load (results in Fetch error) | ||
if (newTraceURL && !newTraceURL.startsWith('blob:')) | ||
setTraceURL(newTraceURL); | ||
}, [setTraceURL]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does it mean useEffect around setTraceURL, wouldn't it run only once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, as the comment says its about if you open trace viewer with a blob URL. Usually old tab, forced reload etc.
31cb34f
to
f7f122e
Compare
f7f122e
to
eea5888
Compare
5eb02a6
to
bd7bc47
Compare
bd7bc47
to
37f9ccc
Compare
This fixes:
npx playwright show-trace non-existing.zip
Fixes #10127