Feature: Add sysproxy to fyne-proxy example
#214
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,7 @@ import ( | |
| "github.com/Jigsaw-Code/outline-sdk/transport" | ||
| "github.com/Jigsaw-Code/outline-sdk/x/config" | ||
| "github.com/Jigsaw-Code/outline-sdk/x/httpproxy" | ||
| "github.com/Jigsaw-Code/outline-sdk/x/sysproxy" | ||
| ) | ||
|
|
||
| type runningProxy struct { | ||
|
|
@@ -126,6 +127,7 @@ func makeAppHeader(title string) *fyne.Container { | |
| } | ||
|
|
||
| func main() { | ||
| defer sysproxy.DisableWebProxy() | ||
| fyneApp := app.New() | ||
| if meta := fyneApp.Metadata(); meta.Name == "" { | ||
| // App not packaged, probably from `go run`. | ||
|
|
@@ -176,7 +178,24 @@ func main() { | |
| if proxy == nil { | ||
| // Start proxy. | ||
| proxy, err = runServer(addressEntry.Text, configEntry.Text) | ||
| if err != nil { | ||
| log.Printf("Failed to start local proxy server: %v\n", err) | ||
| } | ||
| host, port, err := net.SplitHostPort(addressEntry.Text) | ||
| if err != nil { | ||
| log.Printf("Failed to parse address: %v\n", err) | ||
| } | ||
| // Set system-wide proxy settings | ||
| err = sysproxy.SetWebProxy(host, port) | ||
|
There was a problem hiding this comment. Move the logic into runServer. And you need to check if you are on Linux, Windows or macOS in order to set the system proxy. We should output something in the status saying we set the system proxy. There was a problem hiding this comment. @fortuna I will move this to There was a problem hiding this comment. I know, but we should not be calling those methods on the unsupported platforms. There was a problem hiding this comment. when the platform is not supported ( Thinking a bit more about moving
Others can also use the port exposed by the proxy server locally. I can see cases where setting system proxy is not needed. Keeping these logics separate provides extra decoupling. There was a problem hiding this comment. For this application, the Running the proxy commands when it's not supported is a bad practice. In real apps they need to know the support, in order to adapt the UI and application logic. We need a function that tells us whether the System Proxy can be configured. There was a problem hiding this comment. @fortuna I created a new logic called I like your suggestion on having a As a short-term solution I use There was a problem hiding this comment. @fortuna Not sure if you saw the latest changes here. I appreciate your feedback on this whenever you have a moment to review the changes. |
||
| if err != nil { | ||
| log.Printf("Failed to set system wide web proxy settings: %v\n", err) | ||
| } | ||
| } else { | ||
| // Disable system-wide proxy | ||
|
There was a problem hiding this comment. Do this in Close() instead. |
||
| err := sysproxy.DisableWebProxy() | ||
| if err != nil { | ||
| log.Printf("Failed to disable system-wide web proxy: %v\n", err) | ||
| } | ||
| // Stop proxy | ||
| proxy.Close() | ||
| proxy = nil | ||
|
|
||
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.
Disable on runningProxy.Close() instead.
Perhaps have a defer that checks if proxy != nil and closes it.
Move it to where the proxy variable is defined.