fix: simplify browser launch fallback and relay handling
- commit
- ede66e0f3b4bf81e8fbb0d026072735f1fb0d4a3
- author
- brice · · 2026-04-14 17:58 UTC
- parent
- b7da822
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +26 -34
M internal/infrastructure/desktop/adapter.go +13 -2
| @@ -406,7 +406,18 @@ func LaunchExternalURL(uri string) { | |||
| 406 | 406 | ||
| 407 | 407 | // LaunchBrowserURL opens a URL in a new dumber browser window. | |
| 408 | 408 | func LaunchBrowserURL(uri string) { | |
| 409 | - | execPath, err := getExecutablePath() | |
| 409 | + | launchBrowserBrowseURL(uri, getExecutablePath, startDetachedProcess) | |
| 410 | + | } | |
| 411 | + | ||
| 412 | + | func launchBrowserBrowseURL(uri string, resolveExecutablePath func() (string, error), spawnDetachedProcess func(*exec.Cmd) error) { | |
| 413 | + | if resolveExecutablePath == nil { | |
| 414 | + | resolveExecutablePath = getExecutablePath | |
| 415 | + | } | |
| 416 | + | if spawnDetachedProcess == nil { | |
| 417 | + | spawnDetachedProcess = startDetachedProcess | |
| 418 | + | } | |
| 419 | + | ||
| 420 | + | execPath, err := resolveExecutablePath() | |
| 410 | 421 | if err != nil { | |
| 411 | 422 | fmt.Fprintf(os.Stderr, "failed to resolve dumber executable for URL %q: %v\n", uri, err) | |
| 412 | 423 | return | |
| @@ -414,7 +425,7 @@ func LaunchBrowserURL(uri string) { | |||
| 414 | 425 | ||
| 415 | 426 | cmd := exec.Command(execPath, "browse", uri) | |
| 416 | 427 | cmd.Env = sanitizedChildEnv(os.Environ()) | |
| 417 | - | if err := startDetachedProcess(cmd); err != nil { | |
| 428 | + | if err := spawnDetachedProcess(cmd); err != nil { | |
| 418 | 429 | fmt.Fprintf(os.Stderr, "failed to launch dumber browse for URL %q: %v\n", uri, err) | |
| 419 | 430 | } | |
| 420 | 431 | } | |
| @@ -406,7 +406,18 @@ func LaunchExternalURL(uri string) { | |||||
| 406 | 406 | ||||
| 407 | // LaunchBrowserURL opens a URL in a new dumber browser window. | 407 | // LaunchBrowserURL opens a URL in a new dumber browser window. | ||
| 408 | func LaunchBrowserURL(uri string) { | 408 | func LaunchBrowserURL(uri string) { | ||
| 409 | - | execPath, err := getExecutablePath() | 409 | + | launchBrowserBrowseURL(uri, getExecutablePath, startDetachedProcess) |
| 410 | + | } | |||
| 411 | + | ||||
| 412 | + | func launchBrowserBrowseURL(uri string, resolveExecutablePath func() (string, error), spawnDetachedProcess func(*exec.Cmd) error) { | |||
| 413 | + | if resolveExecutablePath == nil { | |||
| 414 | + | resolveExecutablePath = getExecutablePath | |||
| 415 | + | } | |||
| 416 | + | if spawnDetachedProcess == nil { | |||
| 417 | + | spawnDetachedProcess = startDetachedProcess | |||
| 418 | + | } | |||
| 419 | + | ||||
| 420 | + | execPath, err := resolveExecutablePath() | |||
| 410 | if err != nil { | 421 | if err != nil { | ||
| 411 | fmt.Fprintf(os.Stderr, "failed to resolve dumber executable for URL %q: %v\n", uri, err) | 422 | fmt.Fprintf(os.Stderr, "failed to resolve dumber executable for URL %q: %v\n", uri, err) | ||
| 412 | return | 423 | return | ||
| @@ -414,7 +425,7 @@ func LaunchBrowserURL(uri string) { | |||||
| 414 | 425 | ||||
| 415 | cmd := exec.Command(execPath, "browse", uri) | 426 | cmd := exec.Command(execPath, "browse", uri) | ||
| 416 | cmd.Env = sanitizedChildEnv(os.Environ()) | 427 | cmd.Env = sanitizedChildEnv(os.Environ()) | ||
| 417 | - | if err := startDetachedProcess(cmd); err != nil { | 428 | + | if err := spawnDetachedProcess(cmd); err != nil { |
| 418 | fmt.Fprintf(os.Stderr, "failed to launch dumber browse for URL %q: %v\n", uri, err) | 429 | fmt.Fprintf(os.Stderr, "failed to launch dumber browse for URL %q: %v\n", uri, err) | ||
| 419 | } | 430 | } | ||
| 420 | } | 431 | } | ||
M internal/infrastructure/desktop/browser_launch_relay.go +12 -12
| @@ -14,6 +14,7 @@ import ( | |||
| 14 | 14 | "time" | |
| 15 | 15 | ||
| 16 | 16 | "github.com/bnema/dumber/internal/application/port" | |
| 17 | + | "github.com/bnema/dumber/internal/logging" | |
| 17 | 18 | ) | |
| 18 | 19 | ||
| 19 | 20 | const browserLaunchSocketName = "browser-launch.sock" | |
| @@ -31,8 +32,7 @@ type browserLaunchRequest struct { | |||
| 31 | 32 | } | |
| 32 | 33 | ||
| 33 | 34 | type browserLaunchResponse struct { | |
| 34 | - | Delivered bool `json:"delivered"` | |
| 35 | - | Error string `json:"error,omitempty"` | |
| 35 | + | Error string `json:"error,omitempty"` | |
| 36 | 36 | } | |
| 37 | 37 | ||
| 38 | 38 | type browserLaunchRelayListener struct { | |
| @@ -52,13 +52,6 @@ func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url str | |||
| 52 | 52 | return false, err | |
| 53 | 53 | } | |
| 54 | 54 | ||
| 55 | - | if _, statErr := os.Stat(socketPath); statErr != nil { | |
| 56 | - | if os.IsNotExist(statErr) { | |
| 57 | - | return false, nil | |
| 58 | - | } | |
| 59 | - | return false, statErr | |
| 60 | - | } | |
| 61 | - | ||
| 62 | 55 | conn, err := (&net.Dialer{}).DialContext(ctx, "unix", socketPath) | |
| 63 | 56 | if err != nil { | |
| 64 | 57 | if isMissingRelayListener(err) { | |
| @@ -107,7 +100,7 @@ func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url str | |||
| 107 | 100 | } | |
| 108 | 101 | ||
| 109 | 102 | func isMissingRelayListener(err error) bool { | |
| 110 | - | return os.IsNotExist(err) || errors.Is(err, syscall.ECONNREFUSED) | |
| 103 | + | return os.IsNotExist(err) || errors.Is(err, syscall.ECONNREFUSED) || errors.Is(err, syscall.ENOENT) | |
| 111 | 104 | } | |
| 112 | 105 | ||
| 113 | 106 | func isBrowserLaunchReadTimeout(err error) bool { | |
| @@ -220,7 +213,7 @@ func (*browserLaunchRelayListener) handleConnection(ctx context.Context, conn *n | |||
| 220 | 213 | return | |
| 221 | 214 | } | |
| 222 | 215 | ||
| 223 | - | response := browserLaunchResponse{Delivered: true} | |
| 216 | + | response := browserLaunchResponse{} | |
| 224 | 217 | if err := opener.OpenFreshWindow(ctx, request.URL); err != nil { | |
| 225 | 218 | response.Error = err.Error() | |
| 226 | 219 | } | |
| @@ -228,7 +221,14 @@ func (*browserLaunchRelayListener) handleConnection(ctx context.Context, conn *n | |||
| 228 | 221 | if err := conn.SetDeadline(time.Now().Add(browserLaunchIOTimeout)); err != nil { | |
| 229 | 222 | return | |
| 230 | 223 | } | |
| 231 | - | _ = json.NewEncoder(conn).Encode(response) | |
| 224 | + | if err := json.NewEncoder(conn).Encode(response); err != nil { | |
| 225 | + | log := logging.FromContext(ctx) | |
| 226 | + | entry := log.Warn().Err(err).Str("url", request.URL) | |
| 227 | + | if response.Error != "" { | |
| 228 | + | entry = entry.Str("response_error", response.Error) | |
| 229 | + | } | |
| 230 | + | entry.Msg("failed to encode browser launch response") | |
| 231 | + | } | |
| 232 | 232 | } | |
| 233 | 233 | ||
| 234 | 234 | var _ port.BrowserLaunchRelay = (*browserLaunchRelay)(nil) | |
| @@ -14,6 +14,7 @@ import ( | |||||
| 14 | "time" | 14 | "time" | ||
| 15 | 15 | ||||
| 16 | "github.com/bnema/dumber/internal/application/port" | 16 | "github.com/bnema/dumber/internal/application/port" | ||
| 17 | + | "github.com/bnema/dumber/internal/logging" | |||
| 17 | ) | 18 | ) | ||
| 18 | 19 | ||||
| 19 | const browserLaunchSocketName = "browser-launch.sock" | 20 | const browserLaunchSocketName = "browser-launch.sock" | ||
| @@ -31,8 +32,7 @@ type browserLaunchRequest struct { | |||||
| 31 | } | 32 | } | ||
| 32 | 33 | ||||
| 33 | type browserLaunchResponse struct { | 34 | type browserLaunchResponse struct { | ||
| 34 | - | Delivered bool `json:"delivered"` | 35 | + | Error string `json:"error,omitempty"` |
| 35 | - | Error string `json:"error,omitempty"` | |||
| 36 | } | 36 | } | ||
| 37 | 37 | ||||
| 38 | type browserLaunchRelayListener struct { | 38 | type browserLaunchRelayListener struct { | ||
| @@ -52,13 +52,6 @@ func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url str | |||||
| 52 | return false, err | 52 | return false, err | ||
| 53 | } | 53 | } | ||
| 54 | 54 | ||||
| 55 | - | if _, statErr := os.Stat(socketPath); statErr != nil { | |||
| 56 | - | if os.IsNotExist(statErr) { | |||
| 57 | - | return false, nil | |||
| 58 | - | } | |||
| 59 | - | return false, statErr | |||
| 60 | - | } | |||
| 61 | - | ||||
| 62 | conn, err := (&net.Dialer{}).DialContext(ctx, "unix", socketPath) | 55 | conn, err := (&net.Dialer{}).DialContext(ctx, "unix", socketPath) | ||
| 63 | if err != nil { | 56 | if err != nil { | ||
| 64 | if isMissingRelayListener(err) { | 57 | if isMissingRelayListener(err) { | ||
| @@ -107,7 +100,7 @@ func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url str | |||||
| 107 | } | 100 | } | ||
| 108 | 101 | ||||
| 109 | func isMissingRelayListener(err error) bool { | 102 | func isMissingRelayListener(err error) bool { | ||
| 110 | - | return os.IsNotExist(err) || errors.Is(err, syscall.ECONNREFUSED) | 103 | + | return os.IsNotExist(err) || errors.Is(err, syscall.ECONNREFUSED) || errors.Is(err, syscall.ENOENT) |
| 111 | } | 104 | } | ||
| 112 | 105 | ||||
| 113 | func isBrowserLaunchReadTimeout(err error) bool { | 106 | func isBrowserLaunchReadTimeout(err error) bool { | ||
| @@ -220,7 +213,7 @@ func (*browserLaunchRelayListener) handleConnection(ctx context.Context, conn *n | |||||
| 220 | return | 213 | return | ||
| 221 | } | 214 | } | ||
| 222 | 215 | ||||
| 223 | - | response := browserLaunchResponse{Delivered: true} | 216 | + | response := browserLaunchResponse{} |
| 224 | if err := opener.OpenFreshWindow(ctx, request.URL); err != nil { | 217 | if err := opener.OpenFreshWindow(ctx, request.URL); err != nil { | ||
| 225 | response.Error = err.Error() | 218 | response.Error = err.Error() | ||
| 226 | } | 219 | } | ||
| @@ -228,7 +221,14 @@ func (*browserLaunchRelayListener) handleConnection(ctx context.Context, conn *n | |||||
| 228 | if err := conn.SetDeadline(time.Now().Add(browserLaunchIOTimeout)); err != nil { | 221 | if err := conn.SetDeadline(time.Now().Add(browserLaunchIOTimeout)); err != nil { | ||
| 229 | return | 222 | return | ||
| 230 | } | 223 | } | ||
| 231 | - | _ = json.NewEncoder(conn).Encode(response) | 224 | + | if err := json.NewEncoder(conn).Encode(response); err != nil { |
| 225 | + | log := logging.FromContext(ctx) | |||
| 226 | + | entry := log.Warn().Err(err).Str("url", request.URL) | |||
| 227 | + | if response.Error != "" { | |||
| 228 | + | entry = entry.Str("response_error", response.Error) | |||
| 229 | + | } | |||
| 230 | + | entry.Msg("failed to encode browser launch response") | |||
| 231 | + | } | |||
| 232 | } | 232 | } | ||
| 233 | 233 | ||||
| 234 | var _ port.BrowserLaunchRelay = (*browserLaunchRelay)(nil) | 234 | var _ port.BrowserLaunchRelay = (*browserLaunchRelay)(nil) | ||
M internal/infrastructure/desktop/browser_launcher.go +1 -20
| @@ -41,24 +41,5 @@ func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) { | |||
| 41 | 41 | } | |
| 42 | 42 | } | |
| 43 | 43 | ||
| 44 | - | resolveExecutablePath := l.resolveExecutablePath | |
| 45 | - | if resolveExecutablePath == nil { | |
| 46 | - | resolveExecutablePath = getExecutablePath | |
| 47 | - | } | |
| 48 | - | execPath, err := resolveExecutablePath() | |
| 49 | - | if err != nil { | |
| 50 | - | fmt.Fprintf(os.Stderr, "failed to resolve dumber executable for URL %q: %v\n", url, err) | |
| 51 | - | return | |
| 52 | - | } | |
| 53 | - | ||
| 54 | - | cmd := exec.Command(execPath, "browse", url) | |
| 55 | - | cmd.Env = sanitizedChildEnv(os.Environ()) | |
| 56 | - | ||
| 57 | - | spawnDetachedProcess := l.startDetachedProcess | |
| 58 | - | if spawnDetachedProcess == nil { | |
| 59 | - | spawnDetachedProcess = startDetachedProcess | |
| 60 | - | } | |
| 61 | - | if err := spawnDetachedProcess(cmd); err != nil { | |
| 62 | - | fmt.Fprintf(os.Stderr, "failed to launch dumber browse for URL %q: %v\n", url, err) | |
| 63 | - | } | |
| 44 | + | launchBrowserBrowseURL(url, l.resolveExecutablePath, l.startDetachedProcess) | |
| 64 | 45 | } | |
| @@ -41,24 +41,5 @@ func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) { | |||||
| 41 | } | 41 | } | ||
| 42 | } | 42 | } | ||
| 43 | 43 | ||||
| 44 | - | resolveExecutablePath := l.resolveExecutablePath | 44 | + | launchBrowserBrowseURL(url, l.resolveExecutablePath, l.startDetachedProcess) |
| 45 | - | if resolveExecutablePath == nil { | |||
| 46 | - | resolveExecutablePath = getExecutablePath | |||
| 47 | - | } | |||
| 48 | - | execPath, err := resolveExecutablePath() | |||
| 49 | - | if err != nil { | |||
| 50 | - | fmt.Fprintf(os.Stderr, "failed to resolve dumber executable for URL %q: %v\n", url, err) | |||
| 51 | - | return | |||
| 52 | - | } | |||
| 53 | - | ||||
| 54 | - | cmd := exec.Command(execPath, "browse", url) | |||
| 55 | - | cmd.Env = sanitizedChildEnv(os.Environ()) | |||
| 56 | - | ||||
| 57 | - | spawnDetachedProcess := l.startDetachedProcess | |||
| 58 | - | if spawnDetachedProcess == nil { | |||
| 59 | - | spawnDetachedProcess = startDetachedProcess | |||
| 60 | - | } | |||
| 61 | - | if err := spawnDetachedProcess(cmd); err != nil { | |||
| 62 | - | fmt.Fprintf(os.Stderr, "failed to launch dumber browse for URL %q: %v\n", url, err) | |||
| 63 | - | } | |||
| 64 | } | 45 | } | ||