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

Enhance: Refine proxy support; Add proxy support to Logseq Sync #7711

Merged
merged 5 commits into from Dec 19, 2022

Conversation

andelf
Copy link
Collaborator

@andelf andelf commented Dec 13, 2022

  • Add Proxy support for Logseq Sync
  • Use system proxy when possible
  • Use App-scope Proxy for Electron, node-fetch, Logseq Sync
  • Refine proxy settings

image

image

image

@github-actions github-actions bot added the :type/enhancement Enhancement to product. Does not affect the overall basic use. label Dec 13, 2022
@tiensonqin tiensonqin self-requested a review December 14, 2022 04:34
Copy link
Contributor

@tiensonqin tiensonqin left a comment

Choose a reason for hiding this comment

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

LGTM

@andelf andelf changed the title enhance(electron): refine app proxy test url enhance(electron): refine app proxy test Dec 14, 2022
@andelf andelf changed the title enhance(electron): refine app proxy test WIP: enhance(electron): refine app proxy test Dec 14, 2022
@andelf andelf force-pushed the enhance/proxy-selection branch 2 times, most recently from 616665b to 4a44369 Compare December 15, 2022 08:27
@andelf andelf changed the title WIP: enhance(electron): refine app proxy test Enhance: Proxy support for Logseq Sync; refine app proxy support Dec 15, 2022
@andelf andelf requested a review from RCmerci December 15, 2022 08:31
(defmethod handle :testProxyUrl [_win [_ url]]
(let [start-ms (.getTime (js/Date.))]
(-> (utils/fetch url)
(p/timeout 5000)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Need a more reasonable default proxy timeout value here.

(if (and agent-opts (not-empty (:protocol agent-opts)))
(set-fetch-agent agent-opts)
(when-let [sess (.. @*win -webContents -session)]
(p/let [proxy (.resolveProxy sess "https://www.google.com")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The "electron-way" to acquire system proxy settings.
Need a more proper URL here, like https://api.logseq.com or AWS's s3 domain.

(for [{:keys [label value selected]} options]
[:option (cond->
{:key label
:default-value (or value label)}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fix: <option> does not have a default-value.

@andelf andelf changed the title Enhance: Proxy support for Logseq Sync; refine app proxy support WIP: Enhance: Proxy support for Logseq Sync; refine app proxy support Dec 15, 2022
Copy link
Collaborator

@xyhp915 xyhp915 left a comment

Choose a reason for hiding this comment

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

LGTM.

@andelf andelf self-assigned this Dec 16, 2022
@andelf andelf changed the title WIP: Enhance: Proxy support for Logseq Sync; refine app proxy support Enhance: Refine proxy support; Add proxy support to Logseq Sync Dec 16, 2022
@andelf
Copy link
Collaborator Author

andelf commented Dec 16, 2022

@tiensonqin @xyhp915
I have rewritten the proxy setting code. Please review it again. 😁

New changes:

  • Use the same proxy for Electron, node-fetch, and rsapi. (refactored the proxy setting code)
  • Add "System" and "Direct" proxy settings.

Copy link
Collaborator

@cnrpman cnrpman left a comment

Choose a reason for hiding this comment

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

LGTM

@tiensonqin tiensonqin merged commit 6c4dd0b into master Dec 19, 2022
@tiensonqin tiensonqin deleted the enhance/proxy-selection branch December 19, 2022 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:type/enhancement Enhancement to product. Does not affect the overall basic use.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants