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

Feat: Expose HTTPRequest intercept resolution state and clarify docs #7796

Merged
merged 24 commits into from Dec 7, 2021

Conversation

benallfree
Copy link
Contributor

@benallfree benallfree commented Nov 24, 2021

What kind of change does this PR introduce?

New API methods on HTTPRequest

Did you add tests for your changes?

Yes

If relevant, did you update the documentation?

Yes

Summary

#6735 introduced cooperative request intercepts and we are now starting to see adoption. puppeteer-extra just started berstend/puppeteer-extra#592 their support for cooperative mode. In the course of doing so, it became clear that exposing the intercept resolution state would be helpful along with a few doc clarifications.

Does this PR introduce a breaking change?

No

Other information

Requesting review by @jschfflr in particular :)

docs/api.md Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
src/common/HTTPRequest.ts Outdated Show resolved Hide resolved
benallfree and others added 2 commits November 25, 2021 02:10
Co-authored-by: Rodrigo Fernández <fdez.romero@gmail.com>
@google-cla google-cla bot added cla: no and removed cla: yes labels Nov 25, 2021
benallfree and others added 8 commits November 25, 2021 02:16
Co-authored-by: Rodrigo Fernández <fdez.romero@gmail.com>
Co-authored-by: Rodrigo Fernández <fdez.romero@gmail.com>
Co-authored-by: Rodrigo Fernández <fdez.romero@gmail.com>
Co-authored-by: Rodrigo Fernández <fdez.romero@gmail.com>
Co-authored-by: Rodrigo Fernández <fdez.romero@gmail.com>
…/puppeteer into expose-intercept-resolution-2
@benallfree
Copy link
Contributor Author

@FdezRomero Please sign the CLA now that your name is in the commit history :) https://cla.developers.google.com/

@benallfree
Copy link
Contributor Author

benallfree commented Nov 25, 2021

Hello @remusao and @Kikobeats from ghostery/adblocker#2281 and microlinkhq/browserless#321 :)

You are among the first to implement cooperative mode support, I would love to get your feedback/review on this PR.

Copy link
Contributor

@FdezRomero FdezRomero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the improvements 👍🏻

@benallfree
Copy link
Contributor Author

@FdezRomero CI is failing and awaiting your CLA, I wasn’t sure if you saw that.

@FdezRomero
Copy link
Contributor

@benallfree Yes, I did and I signed the CLA, but I can't get the bot to rescan it. I tried with @googlebot rescan but didn't work, so I think it would fix itself if you do another commit.

@google-cla google-cla bot added the cla: yes label Nov 25, 2021
@Tyler-Murphy
Copy link

I'm happy for it to be in any version. Probably needs to be v13 now, since v12 was just released.

@benallfree
Copy link
Contributor Author

benallfree commented Nov 29, 2021

@Tyler-Murphy Spelling error fix moved to #7813 for v13 and removed from this PR so we can do this as a dot release. Thanks for that catch!

@benallfree
Copy link
Contributor Author

@Androbin @OrKoN are either of you able to approve this PR? I’m looking for someone to review it.

@OrKoN
Copy link
Collaborator

OrKoN commented Dec 3, 2021

I will try to find some time to review PRs next week.

@OrKoN OrKoN self-requested a review December 3, 2021 18:33
@OrKoN
Copy link
Collaborator

OrKoN commented Dec 6, 2021

@sadym-chromium could you PTAL as well?

@sadym-chromium
Copy link
Collaborator

Great work, thanks! I just got a bit confused by the Discussion prefix in the Discussion: Cooperative Request Continuation header.

@benallfree
Copy link
Contributor Author

@sadym-chromium Fixed, thank you! 👍

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.

None yet

5 participants