-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
fix: fix failures and correlation in proxy #28094
Conversation
protocolManager: this.protocolManager, | ||
} | ||
|
||
const onError = (error: Error): Promise<void> => { | ||
const pendingRequest = ctx.pendingRequest as PendingRequest | undefined |
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.
This coercion is needed because ctx is HttpMiddlewareCtx<any>
, which causes ctx
to be any
.
Removing this templated any
results in a couple of typescript errors that may be indicative of bugs; one of which is in deferSourceMapRewrite
which should be returning a string instead of void (unless the fn definition on L#320 is more correct than the type definition on L#68). Can resolving the ambiguous definition of ctx
be rolled into this PR, or is that out of scope?
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.
I think at this point given I want to try and get this in for tomorrow, that I'll call this out of scope for now. It's a good find though. This can definitely be cleaned up at some point.
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
We recently made a change to clean up browser prerequests when requests are aborted, but we don't do the same thing with proxy prerequests. This PR ensures that we clean things up on both sides.
Steps to test
I added some integration tests to cover if CDP failures come in first vs. proxy failures.
How has the user experience changed?
n/a
PR Tasks
cypress-documentation
?type definitions
?