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

fix: emit network.responseCompleted for redirects #2098

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

Lightning00Blade
Copy link
Collaborator

@Lightning00Blade Lightning00Blade commented Apr 2, 2024

Currently we never await the responseReceivedExtraInfo as we destroy the BiDi NetworkRequest as soon as we see the redirect happening. This means that the conditions for emitting are never met.
We should eventually change this but having the responseCompleted event with missing headers is better then not having it at all.
Such change would mean that can never have order that matches the expectations
beforeRequestSent -> responseCompleted -> beforeRequestSent
As the responseReceivedExtraInfo comes after the second CDP requestWillBeSent
#2035

@Lightning00Blade Lightning00Blade requested a review from OrKoN April 2, 2024 17:03
@Lightning00Blade Lightning00Blade force-pushed the emit-completed-for-redirects branch from 6c6a497 to 195b055 Compare April 2, 2024 19:53
@Lightning00Blade Lightning00Blade enabled auto-merge (squash) April 2, 2024 19:53
@Lightning00Blade Lightning00Blade merged commit 219cfc9 into main Apr 2, 2024
47 checks passed
@Lightning00Blade Lightning00Blade deleted the emit-completed-for-redirects branch April 2, 2024 20:02
Lightning00Blade pushed a commit that referenced this pull request Apr 10, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.5.17](chromium-bidi-v0.5.16...chromium-bidi-v0.5.17)
(2024-04-10)


### Features

* **network:** support more props for `initiator`
([#2115](#2115))
([80dd9e6](80dd9e6))


### Bug Fixes

* **browsingContext:** emit `navigationFailed` for `navigate` command
failure
([#2118](#2118))
([382a762](382a762))
* don't expect interception for cached events
([#2087](#2087))
([063c1d1](063c1d1))
* emit `network.responseCompleted` for redirects
([#2098](#2098))
([219cfc9](219cfc9))
* **network:** support Interception for OOPIF
([#2110](#2110))
([5d0845c](5d0845c))
* **script:** support PreloadScript in OOPIF
([#2109](#2109))
([baa263e](baa263e))
* sending undefined viewport should keep previously set viewport
([#2119](#2119))
([823e52d](823e52d))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants