Skip to content

Conversation

@deepak1556
Copy link
Member

Description of Change

Follow-up to #40017

Currently there is no way to configure proxy for the net module in the utility process due to network context backend used. Another option would have been to change the network context backend by allowing to configure session property of the net api in the utility process but that is complex since we need to think about how custom protocol and webrequest handlers need to run across process.

Release Notes

Notes: Add proxy configuring support for requests made with net module from utility process

@deepak1556 deepak1556 added semver/minor backwards-compatible functionality target/28-x-y PR should also be added to the "28-x-y" branch. target/29-x-y PR should also be added to the "29-x-y" branch. labels Feb 14, 2024
AIDAI500008

This comment was marked as off-topic.

AIDAI500008

This comment was marked as spam.


**Note:** Enable `Secure Keyboard Entry` only when it is needed and disable it when it is no longer needed.

### `app.setProxy(config)`
Copy link
Member

Choose a reason for hiding this comment

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

Would we want to name this method more descriptively to distinguish it from ses.setProxy? Something like app.setSessionlessProxy or app.setProxyWithoutSession?

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer the consistency in the API names. The scope of the methods clearly define the boundaries between the two methods, also docs are clear on the difference.

@deepak1556 deepak1556 requested a review from ckerr February 15, 2024 00:43
Copy link
Member

@ckerr ckerr left a comment

Choose a reason for hiding this comment

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

...the API seems complex to me, but I can't think of how I would simplify it and keep the same flexibility in proxyRule, i.e. the ability to specify [schemas x URIs x fallbacks]. The RFC-like documentation of exactly what's allowed in proxyRules is helpful 👍

API LGTM. My preference would be to see >2 API reviews on this one before merging.

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label Feb 21, 2024
@github-actions github-actions bot added the target/30-x-y PR should also be added to the "30-x-y" branch. label Feb 21, 2024
Copy link
Member

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

API LGTM

@jkleinsc jkleinsc merged commit 26131b2 into main Feb 22, 2024
@jkleinsc jkleinsc deleted the robo/add_app_proxy_api branch February 22, 2024 17:08
@release-clerk
Copy link

release-clerk bot commented Feb 22, 2024

Release Notes Persisted

Add proxy configuring support for requests made with net module from utility process

@trop
Copy link
Contributor

trop bot commented Feb 22, 2024

I was unable to backport this PR to "28-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Feb 22, 2024

I have automatically backported this PR to "29-x-y", please check out #41416

@trop trop bot added in-flight/29-x-y and removed target/29-x-y PR should also be added to the "29-x-y" branch. labels Feb 22, 2024
@trop
Copy link
Contributor

trop bot commented Feb 22, 2024

I have automatically backported this PR to "30-x-y", please check out #41417

@trop trop bot added in-flight/30-x-y merged/30-x-y PR was merged to the "30-x-y" branch. merged/29-x-y PR was merged to the "29-x-y" branch. and removed target/30-x-y PR should also be added to the "30-x-y" branch. in-flight/30-x-y in-flight/29-x-y labels Feb 22, 2024
@trop
Copy link
Contributor

trop bot commented Mar 29, 2024

@miniak has manually backported this PR to "28-x-y", please check out #41744

jkleinsc pushed a commit that referenced this pull request Apr 3, 2024
)

feat: add support for configuring system network context proxies (#41335)
@trop trop bot added merged/28-x-y PR was merged to the "28-x-y" branch. and removed in-flight/28-x-y labels Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-review/approved ✅ merged/28-x-y PR was merged to the "28-x-y" branch. merged/29-x-y PR was merged to the "29-x-y" branch. merged/30-x-y PR was merged to the "30-x-y" branch. semver/minor backwards-compatible functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants