fix: report relay errors in browser launches
- commit
- a5232ddf2b44a0cbf7aac6942d62ebc9fb6e5303
- author
- brice · · 2026-04-14 15:20 UTC
- parent
- 2c39d19
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +37 -3
M internal/infrastructure/desktop/browser_launcher.go +3 -3
| @@ -33,12 +33,12 @@ func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) { | |||
| 33 | 33 | ||
| 34 | 34 | if l.relay != nil { | |
| 35 | 35 | delivered, err := l.relay.DeliverOpenFreshWindow(ctx, url) | |
| 36 | - | if delivered { | |
| 37 | - | return | |
| 38 | - | } | |
| 39 | 36 | if err != nil { | |
| 40 | 37 | fmt.Fprintf(os.Stderr, "failed to forward dumber browser URL %q: %v\n", url, err) | |
| 41 | 38 | } | |
| 39 | + | if delivered { | |
| 40 | + | return | |
| 41 | + | } | |
| 42 | 42 | } | |
| 43 | 43 | ||
| 44 | 44 | resolveExecutablePath := l.resolveExecutablePath | |
| @@ -33,12 +33,12 @@ func (l *BrowserLauncher) LaunchURL(ctx context.Context, url string) { | |||||
| 33 | 33 | ||||
| 34 | if l.relay != nil { | 34 | if l.relay != nil { | ||
| 35 | delivered, err := l.relay.DeliverOpenFreshWindow(ctx, url) | 35 | delivered, err := l.relay.DeliverOpenFreshWindow(ctx, url) | ||
| 36 | - | if delivered { | |||
| 37 | - | return | |||
| 38 | - | } | |||
| 39 | if err != nil { | 36 | if err != nil { | ||
| 40 | fmt.Fprintf(os.Stderr, "failed to forward dumber browser URL %q: %v\n", url, err) | 37 | fmt.Fprintf(os.Stderr, "failed to forward dumber browser URL %q: %v\n", url, err) | ||
| 41 | } | 38 | } | ||
| 39 | + | if delivered { | |||
| 40 | + | return | |||
| 41 | + | } | |||
| 42 | } | 42 | } | ||
| 43 | 43 | ||||
| 44 | resolveExecutablePath := l.resolveExecutablePath | 44 | resolveExecutablePath := l.resolveExecutablePath | ||
M internal/infrastructure/desktop/browser_launcher_test.go +34 -0
| @@ -2,6 +2,9 @@ package desktop | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "errors" | |
| 6 | + | "io" | |
| 7 | + | "os" | |
| 5 | 8 | "os/exec" | |
| 6 | 9 | "testing" | |
| 7 | 10 | ||
| @@ -51,3 +54,34 @@ func TestBrowserLauncher_LaunchURL_FallsBackToSpawnWhenRelayMisses(t *testing.T) | |||
| 51 | 54 | assert.Equal(t, "/usr/bin/dumber", gotCmd.Path) | |
| 52 | 55 | assert.Equal(t, []string{"/usr/bin/dumber", "browse", "https://example.com"}, gotCmd.Args) | |
| 53 | 56 | } | |
| 57 | + | ||
| 58 | + | func TestBrowserLauncher_LaunchURL_ReportsRelayErrorWhenDelivered(t *testing.T) { | |
| 59 | + | relay := mocks.NewMockBrowserLaunchRelay(t) | |
| 60 | + | wantErr := errors.New("relay exploded") | |
| 61 | + | relay.EXPECT().DeliverOpenFreshWindow(context.Background(), "https://example.com").Return(true, wantErr) | |
| 62 | + | ||
| 63 | + | launcher := NewBrowserLauncher(relay) | |
| 64 | + | launcher.resolveExecutablePath = func() (string, error) { | |
| 65 | + | t.Fatal("expected executable path resolution to be skipped") | |
| 66 | + | return "", nil | |
| 67 | + | } | |
| 68 | + | launcher.startDetachedProcess = func(cmd *exec.Cmd) error { | |
| 69 | + | t.Fatal("expected launch to stop after relay delivery") | |
| 70 | + | return nil | |
| 71 | + | } | |
| 72 | + | ||
| 73 | + | oldStderr := os.Stderr | |
| 74 | + | r, w, err := os.Pipe() | |
| 75 | + | require.NoError(t, err) | |
| 76 | + | os.Stderr = w | |
| 77 | + | t.Cleanup(func() { | |
| 78 | + | os.Stderr = oldStderr | |
| 79 | + | }) | |
| 80 | + | ||
| 81 | + | launcher.LaunchURL(context.Background(), "https://example.com") | |
| 82 | + | ||
| 83 | + | require.NoError(t, w.Close()) | |
| 84 | + | output, err := io.ReadAll(r) | |
| 85 | + | require.NoError(t, err) | |
| 86 | + | assert.Contains(t, string(output), wantErr.Error()) | |
| 87 | + | } | |
| @@ -2,6 +2,9 @@ package desktop | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "errors" | |||
| 6 | + | "io" | |||
| 7 | + | "os" | |||
| 5 | "os/exec" | 8 | "os/exec" | ||
| 6 | "testing" | 9 | "testing" | ||
| 7 | 10 | ||||
| @@ -51,3 +54,34 @@ func TestBrowserLauncher_LaunchURL_FallsBackToSpawnWhenRelayMisses(t *testing.T) | |||||
| 51 | assert.Equal(t, "/usr/bin/dumber", gotCmd.Path) | 54 | assert.Equal(t, "/usr/bin/dumber", gotCmd.Path) | ||
| 52 | assert.Equal(t, []string{"/usr/bin/dumber", "browse", "https://example.com"}, gotCmd.Args) | 55 | assert.Equal(t, []string{"/usr/bin/dumber", "browse", "https://example.com"}, gotCmd.Args) | ||
| 53 | } | 56 | } | ||
| 57 | + | ||||
| 58 | + | func TestBrowserLauncher_LaunchURL_ReportsRelayErrorWhenDelivered(t *testing.T) { | |||
| 59 | + | relay := mocks.NewMockBrowserLaunchRelay(t) | |||
| 60 | + | wantErr := errors.New("relay exploded") | |||
| 61 | + | relay.EXPECT().DeliverOpenFreshWindow(context.Background(), "https://example.com").Return(true, wantErr) | |||
| 62 | + | ||||
| 63 | + | launcher := NewBrowserLauncher(relay) | |||
| 64 | + | launcher.resolveExecutablePath = func() (string, error) { | |||
| 65 | + | t.Fatal("expected executable path resolution to be skipped") | |||
| 66 | + | return "", nil | |||
| 67 | + | } | |||
| 68 | + | launcher.startDetachedProcess = func(cmd *exec.Cmd) error { | |||
| 69 | + | t.Fatal("expected launch to stop after relay delivery") | |||
| 70 | + | return nil | |||
| 71 | + | } | |||
| 72 | + | ||||
| 73 | + | oldStderr := os.Stderr | |||
| 74 | + | r, w, err := os.Pipe() | |||
| 75 | + | require.NoError(t, err) | |||
| 76 | + | os.Stderr = w | |||
| 77 | + | t.Cleanup(func() { | |||
| 78 | + | os.Stderr = oldStderr | |||
| 79 | + | }) | |||
| 80 | + | ||||
| 81 | + | launcher.LaunchURL(context.Background(), "https://example.com") | |||
| 82 | + | ||||
| 83 | + | require.NoError(t, w.Close()) | |||
| 84 | + | output, err := io.ReadAll(r) | |||
| 85 | + | require.NoError(t, err) | |||
| 86 | + | assert.Contains(t, string(output), wantErr.Error()) | |||
| 87 | + | } | |||