chore(webkit): update error stack parsing and related system tests #23730
Conversation
|
Thanks for taking the time to open a PR!
|
| errStack = errStack.replaceAll(webkitStackLineRegex, (match, ...parts: string[]) => { | ||
| // We patch WebKit's Error within the AUT as CyWebKitError, causing it to | ||
| // be presented within the stack. If we detect it within the stack, we remove it. | ||
| if (parts[0] === '__CyWebKitError') { |
There was a problem hiding this comment.
Is this because it's showing up in the stack trace of every new Error? Should we just not name the function then?
There was a problem hiding this comment.
I went back and forth on this. If we don't name it, it'll still render here, just as yet another anonymous '' function. Given the frequency with which it'll be presented (for any uncaught exception thrown by the AUT), I figured its presence in the stacks might be confusing, named or not. But I could understand the argument that removing it is an improper obfuscation of what is actually being executed?
There was a problem hiding this comment.
hmm, yeah... maybe we just oughtta name it Error, then. This seems fine too. Not too big an issue for experimental.
|
Test summary
Run details
View run in Cypress Dashboard ➡️ Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
||||||||||||||||||||||||||
|
Ahhhhhh I also have an open PR #23575 that messes with stack_utils, MAY THE GIT MERGE BEGIN!! Since my stuff is annoying and a lil complicated and I know how to test it, let's merge yours first and I'll figure out the conflict 🥲 ⚔️ |
This reverts commit 2b91eff.
…out function name
User facing changelog
N/A
Additional details
WebKit error stacks currently have a number of documented issues, resulting in stack traces that aren't very helpful:
https://bugs.webkit.org/show_bug.cgi?id=86493
https://bugs.webkit.org/show_bug.cgi?id=243668
https://bugs.webkit.org/show_bug.cgi?id=174380
This PR updates WebKit stack lines with some placeholder function/location values, which makes them more readable (if not more functional) and allows us to better parse them for presentation within the reporter and snapshot normalization. We are using
<unknown>in the existing parsing logic when function names/locations could not be determined and I continued that here, but we could use something more verbose if it would be helpful.It's important to note that the lack of call locations in these stacks makes a lot of our existing strategies for stack inspection/replacement much harder (if not impossible) to implement. A number of changes have recently landed in WebKit that should improve these stacks, at least so far as removing a lot of these internal stack items that this PR is working with. But the full impact of these changes (and the timeline for their inclusion in a new playwright-webkit release) is yet to be determined.
Steps to test
expect(true).to.eq(false), throw uncaught error in fixture, etc.)How has the user experience changed?
Before:
After:
PR Tasks
cypress-documentation?type definitions?