feat(daemon): wrap pane and tab spawn failures as user errors
createTab, spawnPaneOp's pty-open failure, its mutate/Solve layout.ErrTooSmall paths, and toggleFloating's structural errors now return domain.UserError with the precise notice code (NoticeTabSpawn, NoticePaneSpawn, NoticeLayoutTooSmall, NoticeFloatingSpawn) instead of the NoticeInternal catch-all, so d.reportError's palette funnel (Task 7) surfaces a specific, actionable toast.
- commit
- bdb821c9007f76f0f650fdb21d251f5644ecf65b
- author
- brice · · 2026-07-20 17:47 UTC
- parent
- a4b2aa9
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +80 -13
M internal/usecase/daemon/floating.go +2 -3
| @@ -2,7 +2,6 @@ package daemon | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | - | "fmt" | |
| 6 | 5 | "path/filepath" | |
| 7 | 6 | "strings" | |
| 8 | 7 | ||
| @@ -192,11 +191,11 @@ func (d *Daemon) activateTabAfterResize(sess *session, tb *tab, outerResizeAccep | |||
| 192 | 191 | // launches asynchronously; installed slots are retained when hidden. | |
| 193 | 192 | func (d *Daemon) toggleFloating(sess *session, ac *attachedClient) error { | |
| 194 | 193 | if d == nil || sess == nil { | |
| 195 | - | return fmt.Errorf("floating pane: session required") | |
| 194 | + | return domain.UserErr(domain.NoticeFloatingSpawn, "couldn't open floating pane: no active session", nil) | |
| 196 | 195 | } | |
| 197 | 196 | tb := sess.activeTab() | |
| 198 | 197 | if tb == nil { | |
| 199 | - | return layout.ErrNotFound | |
| 198 | + | return domain.UserErr(domain.NoticeFloatingSpawn, "couldn't open floating pane: no active tab", layout.ErrNotFound) | |
| 200 | 199 | } | |
| 201 | 200 | cfg := d.currentFloatingConfig() | |
| 202 | 201 | tb.mu.Lock() | |
| @@ -2,7 +2,6 @@ package daemon | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | - | "fmt" | |||
| 6 | "path/filepath" | 5 | "path/filepath" | ||
| 7 | "strings" | 6 | "strings" | ||
| 8 | 7 | ||||
| @@ -192,11 +191,11 @@ func (d *Daemon) activateTabAfterResize(sess *session, tb *tab, outerResizeAccep | |||||
| 192 | // launches asynchronously; installed slots are retained when hidden. | 191 | // launches asynchronously; installed slots are retained when hidden. | ||
| 193 | func (d *Daemon) toggleFloating(sess *session, ac *attachedClient) error { | 192 | func (d *Daemon) toggleFloating(sess *session, ac *attachedClient) error { | ||
| 194 | if d == nil || sess == nil { | 193 | if d == nil || sess == nil { | ||
| 195 | - | return fmt.Errorf("floating pane: session required") | 194 | + | return domain.UserErr(domain.NoticeFloatingSpawn, "couldn't open floating pane: no active session", nil) |
| 196 | } | 195 | } | ||
| 197 | tb := sess.activeTab() | 196 | tb := sess.activeTab() | ||
| 198 | if tb == nil { | 197 | if tb == nil { | ||
| 199 | - | return layout.ErrNotFound | 198 | + | return domain.UserErr(domain.NoticeFloatingSpawn, "couldn't open floating pane: no active tab", layout.ErrNotFound) |
| 200 | } | 199 | } | ||
| 201 | cfg := d.currentFloatingConfig() | 200 | cfg := d.currentFloatingConfig() | ||
| 202 | tb.mu.Lock() | 201 | tb.mu.Lock() | ||
M internal/usecase/daemon/floating_test.go +23 -0
| @@ -592,6 +592,29 @@ func TestFloatingInstallLogsSessionNameSafelyDuringRename(t *testing.T) { | |||
| 592 | 592 | d.sessWg.Wait() | |
| 593 | 593 | } | |
| 594 | 594 | ||
| 595 | + | func TestToggleFloatingStructuralErrorsAreUserErrors(t *testing.T) { | |
| 596 | + | tests := []struct { | |
| 597 | + | name string | |
| 598 | + | d *Daemon | |
| 599 | + | sess *session | |
| 600 | + | }{ | |
| 601 | + | {"nil session", newTestDaemon(t, nil, stubClock{}), nil}, | |
| 602 | + | {"no active tab", newTestDaemon(t, nil, stubClock{}), &session{}}, | |
| 603 | + | } | |
| 604 | + | for _, tc := range tests { | |
| 605 | + | t.Run(tc.name, func(t *testing.T) { | |
| 606 | + | err := tc.d.toggleFloating(tc.sess, nil) | |
| 607 | + | require.Error(t, err) | |
| 608 | + | ||
| 609 | + | var ue *domain.UserError | |
| 610 | + | require.ErrorAs(t, err, &ue) | |
| 611 | + | require.Equal(t, domain.NoticeFloatingSpawn, ue.Code) | |
| 612 | + | require.Equal(t, domain.NoticeError, ue.Severity) | |
| 613 | + | require.NotEmpty(t, ue.Msg) | |
| 614 | + | }) | |
| 615 | + | } | |
| 616 | + | } | |
| 617 | + | ||
| 595 | 618 | func TestFloatingToggleUsesVisibleTarget(t *testing.T) { | |
| 596 | 619 | d := newTestDaemon(t, nil, stubClock{}) | |
| 597 | 620 | tb := newFloatingTestTab(t) | |
| @@ -592,6 +592,29 @@ func TestFloatingInstallLogsSessionNameSafelyDuringRename(t *testing.T) { | |||||
| 592 | d.sessWg.Wait() | 592 | d.sessWg.Wait() | ||
| 593 | } | 593 | } | ||
| 594 | 594 | ||||
| 595 | + | func TestToggleFloatingStructuralErrorsAreUserErrors(t *testing.T) { | |||
| 596 | + | tests := []struct { | |||
| 597 | + | name string | |||
| 598 | + | d *Daemon | |||
| 599 | + | sess *session | |||
| 600 | + | }{ | |||
| 601 | + | {"nil session", newTestDaemon(t, nil, stubClock{}), nil}, | |||
| 602 | + | {"no active tab", newTestDaemon(t, nil, stubClock{}), &session{}}, | |||
| 603 | + | } | |||
| 604 | + | for _, tc := range tests { | |||
| 605 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 606 | + | err := tc.d.toggleFloating(tc.sess, nil) | |||
| 607 | + | require.Error(t, err) | |||
| 608 | + | ||||
| 609 | + | var ue *domain.UserError | |||
| 610 | + | require.ErrorAs(t, err, &ue) | |||
| 611 | + | require.Equal(t, domain.NoticeFloatingSpawn, ue.Code) | |||
| 612 | + | require.Equal(t, domain.NoticeError, ue.Severity) | |||
| 613 | + | require.NotEmpty(t, ue.Msg) | |||
| 614 | + | }) | |||
| 615 | + | } | |||
| 616 | + | } | |||
| 617 | + | ||||
| 595 | func TestFloatingToggleUsesVisibleTarget(t *testing.T) { | 618 | func TestFloatingToggleUsesVisibleTarget(t *testing.T) { | ||
| 596 | d := newTestDaemon(t, nil, stubClock{}) | 619 | d := newTestDaemon(t, nil, stubClock{}) | ||
| 597 | tb := newFloatingTestTab(t) | 620 | tb := newFloatingTestTab(t) | ||
M internal/usecase/daemon/palette_test.go +4 -4
| @@ -121,9 +121,9 @@ func TestPaletteOpenTypeEnterRunAndEscClose(t *testing.T) { | |||
| 121 | 121 | ||
| 122 | 122 | // TestPaletteCommandFailureSurfacesAsNotice drives a palette command whose Run | |
| 123 | 123 | // target fails and asserts the failure reaches the user as a notice instead of | |
| 124 | - | // only a log line. Task 8 will wrap the new-tab spawn failure as a | |
| 125 | - | // domain.UserError with NoticeTabSpawn; until then it flows through | |
| 126 | - | // unclassified and lands on the NoticeInternal catch-all. | |
| 124 | + | // only a log line. The new-tab spawn failure is wrapped as a domain.UserError | |
| 125 | + | // with NoticeTabSpawn (session.go createTab), so it surfaces under that code | |
| 126 | + | // rather than the NoticeInternal catch-all. | |
| 127 | 127 | func TestPaletteCommandFailureSurfacesAsNotice(t *testing.T) { | |
| 128 | 128 | p, release := newBlockingPTY(t) | |
| 129 | 129 | defer release() | |
| @@ -139,7 +139,7 @@ func TestPaletteCommandFailureSurfacesAsNotice(t *testing.T) { | |||
| 139 | 139 | ||
| 140 | 140 | history := d.notices.history() | |
| 141 | 141 | require.NotEmpty(t, history, "failed palette command must record a notice") | |
| 142 | - | require.Equal(t, domain.NoticeInternal, history[0].Code) | |
| 142 | + | require.Equal(t, domain.NoticeTabSpawn, history[0].Code) | |
| 143 | 143 | } | |
| 144 | 144 | ||
| 145 | 145 | func TestPaletteEntryPublishesEligibleNamedSessionResults(t *testing.T) { | |
| @@ -121,9 +121,9 @@ func TestPaletteOpenTypeEnterRunAndEscClose(t *testing.T) { | |||||
| 121 | 121 | ||||
| 122 | // TestPaletteCommandFailureSurfacesAsNotice drives a palette command whose Run | 122 | // TestPaletteCommandFailureSurfacesAsNotice drives a palette command whose Run | ||
| 123 | // target fails and asserts the failure reaches the user as a notice instead of | 123 | // target fails and asserts the failure reaches the user as a notice instead of | ||
| 124 | - | // only a log line. Task 8 will wrap the new-tab spawn failure as a | 124 | + | // only a log line. The new-tab spawn failure is wrapped as a domain.UserError |
| 125 | - | // domain.UserError with NoticeTabSpawn; until then it flows through | 125 | + | // with NoticeTabSpawn (session.go createTab), so it surfaces under that code |
| 126 | - | // unclassified and lands on the NoticeInternal catch-all. | 126 | + | // rather than the NoticeInternal catch-all. |
| 127 | func TestPaletteCommandFailureSurfacesAsNotice(t *testing.T) { | 127 | func TestPaletteCommandFailureSurfacesAsNotice(t *testing.T) { | ||
| 128 | p, release := newBlockingPTY(t) | 128 | p, release := newBlockingPTY(t) | ||
| 129 | defer release() | 129 | defer release() | ||
| @@ -139,7 +139,7 @@ func TestPaletteCommandFailureSurfacesAsNotice(t *testing.T) { | |||||
| 139 | 139 | ||||
| 140 | history := d.notices.history() | 140 | history := d.notices.history() | ||
| 141 | require.NotEmpty(t, history, "failed palette command must record a notice") | 141 | require.NotEmpty(t, history, "failed palette command must record a notice") | ||
| 142 | - | require.Equal(t, domain.NoticeInternal, history[0].Code) | 142 | + | require.Equal(t, domain.NoticeTabSpawn, history[0].Code) |
| 143 | } | 143 | } | ||
| 144 | 144 | ||||
| 145 | func TestPaletteEntryPublishesEligibleNamedSessionResults(t *testing.T) { | 145 | func TestPaletteEntryPublishesEligibleNamedSessionResults(t *testing.T) { | ||
M internal/usecase/daemon/pane_actions.go +5 -2
| @@ -48,6 +48,9 @@ func (d *Daemon) spawnPaneOp( | |||
| 48 | 48 | area := domain.Rect{Width: tb.size.Cols, Height: tb.size.Rows} | |
| 49 | 49 | if err := mutate(tb.tree, oldFocus, newID, area); err != nil { | |
| 50 | 50 | tb.mu.Unlock() | |
| 51 | + | if errors.Is(err, layout.ErrTooSmall) { | |
| 52 | + | return domain.UserWarn(domain.NoticeLayoutTooSmall, "not enough space to split", err) | |
| 53 | + | } | |
| 51 | 54 | return err | |
| 52 | 55 | } | |
| 53 | 56 | tb.nextPaneID++ | |
| @@ -56,7 +59,7 @@ func (d *Daemon) spawnPaneOp( | |||
| 56 | 59 | _ = tb.tree.Close(newID) | |
| 57 | 60 | tb.tree.Focus = oldFocus | |
| 58 | 61 | tb.mu.Unlock() | |
| 59 | - | return layout.ErrTooSmall | |
| 62 | + | return domain.UserWarn(domain.NoticeLayoutTooSmall, "not enough space to split", layout.ErrTooSmall) | |
| 60 | 63 | } | |
| 61 | 64 | newRect := placementContent(placements, newID) | |
| 62 | 65 | tabStableID := tb.stableID | |
| @@ -78,7 +81,7 @@ func (d *Daemon) spawnPaneOp( | |||
| 78 | 81 | _ = tb.tree.Close(newID) | |
| 79 | 82 | tb.tree.Focus = oldFocus | |
| 80 | 83 | tb.mu.Unlock() | |
| 81 | - | return err | |
| 84 | + | return domain.UserErr(domain.NoticePaneSpawn, "couldn't open pane: shell failed to start", err) | |
| 82 | 85 | } | |
| 83 | 86 | ||
| 84 | 87 | pctx, cancel := context.WithCancel(tb.ctx) | |
| @@ -48,6 +48,9 @@ func (d *Daemon) spawnPaneOp( | |||||
| 48 | area := domain.Rect{Width: tb.size.Cols, Height: tb.size.Rows} | 48 | area := domain.Rect{Width: tb.size.Cols, Height: tb.size.Rows} | ||
| 49 | if err := mutate(tb.tree, oldFocus, newID, area); err != nil { | 49 | if err := mutate(tb.tree, oldFocus, newID, area); err != nil { | ||
| 50 | tb.mu.Unlock() | 50 | tb.mu.Unlock() | ||
| 51 | + | if errors.Is(err, layout.ErrTooSmall) { | |||
| 52 | + | return domain.UserWarn(domain.NoticeLayoutTooSmall, "not enough space to split", err) | |||
| 53 | + | } | |||
| 51 | return err | 54 | return err | ||
| 52 | } | 55 | } | ||
| 53 | tb.nextPaneID++ | 56 | tb.nextPaneID++ | ||
| @@ -56,7 +59,7 @@ func (d *Daemon) spawnPaneOp( | |||||
| 56 | _ = tb.tree.Close(newID) | 59 | _ = tb.tree.Close(newID) | ||
| 57 | tb.tree.Focus = oldFocus | 60 | tb.tree.Focus = oldFocus | ||
| 58 | tb.mu.Unlock() | 61 | tb.mu.Unlock() | ||
| 59 | - | return layout.ErrTooSmall | 62 | + | return domain.UserWarn(domain.NoticeLayoutTooSmall, "not enough space to split", layout.ErrTooSmall) |
| 60 | } | 63 | } | ||
| 61 | newRect := placementContent(placements, newID) | 64 | newRect := placementContent(placements, newID) | ||
| 62 | tabStableID := tb.stableID | 65 | tabStableID := tb.stableID | ||
| @@ -78,7 +81,7 @@ func (d *Daemon) spawnPaneOp( | |||||
| 78 | _ = tb.tree.Close(newID) | 81 | _ = tb.tree.Close(newID) | ||
| 79 | tb.tree.Focus = oldFocus | 82 | tb.tree.Focus = oldFocus | ||
| 80 | tb.mu.Unlock() | 83 | tb.mu.Unlock() | ||
| 81 | - | return err | 84 | + | return domain.UserErr(domain.NoticePaneSpawn, "couldn't open pane: shell failed to start", err) |
| 82 | } | 85 | } | ||
| 83 | 86 | ||||
| 84 | pctx, cancel := context.WithCancel(tb.ctx) | 87 | pctx, cancel := context.WithCancel(tb.ctx) | ||
M internal/usecase/daemon/pane_actions_test.go +21 -3
| @@ -85,14 +85,24 @@ func TestSplitPaneRightFromStackSplitsWholeStack(t *testing.T) { | |||
| 85 | 85 | ||
| 86 | 86 | func TestSplitPaneOpenErrorRollsBackTreeAndPaneMap(t *testing.T) { | |
| 87 | 87 | d, sess, _, factory := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | |
| 88 | - | factory.EXPECT().Open(mock.Anything, "/bin/sh", []string(nil), mock.Anything, "/work", domain.Size{Cols: 20, Rows: 10}).Return(nil, errors.New("open failed")).Once() | |
| 88 | + | cause := errors.New("open failed") | |
| 89 | + | factory.EXPECT().Open(mock.Anything, "/bin/sh", []string(nil), mock.Anything, "/work", domain.Size{Cols: 20, Rows: 10}).Return(nil, cause).Once() | |
| 89 | 90 | ||
| 90 | 91 | tb := sess.activeTab() | |
| 91 | 92 | tb.mu.Lock() | |
| 92 | 93 | beforeRoot, beforeFocus, beforeNext, beforePaneCount := tb.tree.Root.Leaf, tb.tree.Focus, tb.nextPaneID, len(tb.panes) | |
| 93 | 94 | tb.mu.Unlock() | |
| 94 | 95 | ||
| 95 | - | require.Error(t, d.splitPane(sess, nil, layout.Right)) | |
| 96 | + | err := d.splitPane(sess, nil, layout.Right) | |
| 97 | + | require.Error(t, err) | |
| 98 | + | ||
| 99 | + | var ue *domain.UserError | |
| 100 | + | require.ErrorAs(t, err, &ue) | |
| 101 | + | require.Equal(t, domain.NoticePaneSpawn, ue.Code) | |
| 102 | + | require.Equal(t, domain.NoticeError, ue.Severity) | |
| 103 | + | require.Equal(t, "couldn't open pane: shell failed to start", ue.Msg) | |
| 104 | + | require.ErrorIs(t, err, cause) | |
| 105 | + | require.NotContains(t, ue.Msg, cause.Error()) | |
| 96 | 106 | ||
| 97 | 107 | tb.mu.Lock() | |
| 98 | 108 | defer tb.mu.Unlock() | |
| @@ -263,8 +273,16 @@ func TestStackFocusWalkExpandsAndOverflowRefuses(t *testing.T) { | |||
| 263 | 273 | require.Equal(t, layout.PaneID("pane-1"), tb.tree.Root.Expanded) | |
| 264 | 274 | thirdPTY := portsmocks.NewMockPTY(t) | |
| 265 | 275 | factory.EXPECT().Open(mock.Anything, "/bin/sh", []string(nil), mock.Anything, "/work", mock.Anything).Return(thirdPTY, nil).Maybe() | |
| 266 | - | require.ErrorIs(t, d.stackPane(sess, nil), layout.ErrTooSmall) | |
| 276 | + | err := d.stackPane(sess, nil) | |
| 277 | + | require.ErrorIs(t, err, layout.ErrTooSmall) | |
| 267 | 278 | require.Len(t, tb.panes, 2) | |
| 279 | + | ||
| 280 | + | var ue *domain.UserError | |
| 281 | + | require.ErrorAs(t, err, &ue) | |
| 282 | + | require.Equal(t, domain.NoticeLayoutTooSmall, ue.Code) | |
| 283 | + | require.Equal(t, domain.NoticeWarn, ue.Severity) | |
| 284 | + | require.Equal(t, "not enough space to split", ue.Msg) | |
| 285 | + | require.NotContains(t, ue.Msg, layout.ErrTooSmall.Error()) | |
| 268 | 286 | } | |
| 269 | 287 | ||
| 270 | 288 | func newSplitTestDaemon(t *testing.T, sz domain.Size) (*Daemon, *session, *portsmocks.MockPTY, *portsmocks.MockPTYFactory) { | |
| @@ -85,14 +85,24 @@ func TestSplitPaneRightFromStackSplitsWholeStack(t *testing.T) { | |||||
| 85 | 85 | ||||
| 86 | func TestSplitPaneOpenErrorRollsBackTreeAndPaneMap(t *testing.T) { | 86 | func TestSplitPaneOpenErrorRollsBackTreeAndPaneMap(t *testing.T) { | ||
| 87 | d, sess, _, factory := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | 87 | d, sess, _, factory := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | ||
| 88 | - | factory.EXPECT().Open(mock.Anything, "/bin/sh", []string(nil), mock.Anything, "/work", domain.Size{Cols: 20, Rows: 10}).Return(nil, errors.New("open failed")).Once() | 88 | + | cause := errors.New("open failed") |
| 89 | + | factory.EXPECT().Open(mock.Anything, "/bin/sh", []string(nil), mock.Anything, "/work", domain.Size{Cols: 20, Rows: 10}).Return(nil, cause).Once() | |||
| 89 | 90 | ||||
| 90 | tb := sess.activeTab() | 91 | tb := sess.activeTab() | ||
| 91 | tb.mu.Lock() | 92 | tb.mu.Lock() | ||
| 92 | beforeRoot, beforeFocus, beforeNext, beforePaneCount := tb.tree.Root.Leaf, tb.tree.Focus, tb.nextPaneID, len(tb.panes) | 93 | beforeRoot, beforeFocus, beforeNext, beforePaneCount := tb.tree.Root.Leaf, tb.tree.Focus, tb.nextPaneID, len(tb.panes) | ||
| 93 | tb.mu.Unlock() | 94 | tb.mu.Unlock() | ||
| 94 | 95 | ||||
| 95 | - | require.Error(t, d.splitPane(sess, nil, layout.Right)) | 96 | + | err := d.splitPane(sess, nil, layout.Right) |
| 97 | + | require.Error(t, err) | |||
| 98 | + | ||||
| 99 | + | var ue *domain.UserError | |||
| 100 | + | require.ErrorAs(t, err, &ue) | |||
| 101 | + | require.Equal(t, domain.NoticePaneSpawn, ue.Code) | |||
| 102 | + | require.Equal(t, domain.NoticeError, ue.Severity) | |||
| 103 | + | require.Equal(t, "couldn't open pane: shell failed to start", ue.Msg) | |||
| 104 | + | require.ErrorIs(t, err, cause) | |||
| 105 | + | require.NotContains(t, ue.Msg, cause.Error()) | |||
| 96 | 106 | ||||
| 97 | tb.mu.Lock() | 107 | tb.mu.Lock() | ||
| 98 | defer tb.mu.Unlock() | 108 | defer tb.mu.Unlock() | ||
| @@ -263,8 +273,16 @@ func TestStackFocusWalkExpandsAndOverflowRefuses(t *testing.T) { | |||||
| 263 | require.Equal(t, layout.PaneID("pane-1"), tb.tree.Root.Expanded) | 273 | require.Equal(t, layout.PaneID("pane-1"), tb.tree.Root.Expanded) | ||
| 264 | thirdPTY := portsmocks.NewMockPTY(t) | 274 | thirdPTY := portsmocks.NewMockPTY(t) | ||
| 265 | factory.EXPECT().Open(mock.Anything, "/bin/sh", []string(nil), mock.Anything, "/work", mock.Anything).Return(thirdPTY, nil).Maybe() | 275 | factory.EXPECT().Open(mock.Anything, "/bin/sh", []string(nil), mock.Anything, "/work", mock.Anything).Return(thirdPTY, nil).Maybe() | ||
| 266 | - | require.ErrorIs(t, d.stackPane(sess, nil), layout.ErrTooSmall) | 276 | + | err := d.stackPane(sess, nil) |
| 277 | + | require.ErrorIs(t, err, layout.ErrTooSmall) | |||
| 267 | require.Len(t, tb.panes, 2) | 278 | require.Len(t, tb.panes, 2) | ||
| 279 | + | ||||
| 280 | + | var ue *domain.UserError | |||
| 281 | + | require.ErrorAs(t, err, &ue) | |||
| 282 | + | require.Equal(t, domain.NoticeLayoutTooSmall, ue.Code) | |||
| 283 | + | require.Equal(t, domain.NoticeWarn, ue.Severity) | |||
| 284 | + | require.Equal(t, "not enough space to split", ue.Msg) | |||
| 285 | + | require.NotContains(t, ue.Msg, layout.ErrTooSmall.Error()) | |||
| 268 | } | 286 | } | ||
| 269 | 287 | ||||
| 270 | func newSplitTestDaemon(t *testing.T, sz domain.Size) (*Daemon, *session, *portsmocks.MockPTY, *portsmocks.MockPTYFactory) { | 288 | func newSplitTestDaemon(t *testing.T, sz domain.Size) (*Daemon, *session, *portsmocks.MockPTY, *portsmocks.MockPTYFactory) { | ||
M internal/usecase/daemon/session.go +1 -1
| @@ -309,7 +309,7 @@ func (d *Daemon) createTab(sess *session, sz domain.Size) error { | |||
| 309 | 309 | pty, err := d.ptys.Open(sess.ctx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, tbSize) | |
| 310 | 310 | if err != nil { | |
| 311 | 311 | d.log.Warn("pty spawn failed", "err", err, "session", name, "kind", "tab") | |
| 312 | - | return fmt.Errorf("daemon: spawning tab for session %q: %w", name, err) | |
| 312 | + | return domain.UserErr(domain.NoticeTabSpawn, "couldn't open tab: shell failed to start", err) | |
| 313 | 313 | } | |
| 314 | 314 | tb := newTabWithStableID(tabStableID, paneStableID, pty, tbSize) | |
| 315 | 315 | if client != nil { | |
| @@ -309,7 +309,7 @@ func (d *Daemon) createTab(sess *session, sz domain.Size) error { | |||||
| 309 | pty, err := d.ptys.Open(sess.ctx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, tbSize) | 309 | pty, err := d.ptys.Open(sess.ctx, command, args, childEnvFrom(env, name, tabStableID, paneStableID, term), cwd, tbSize) | ||
| 310 | if err != nil { | 310 | if err != nil { | ||
| 311 | d.log.Warn("pty spawn failed", "err", err, "session", name, "kind", "tab") | 311 | d.log.Warn("pty spawn failed", "err", err, "session", name, "kind", "tab") | ||
| 312 | - | return fmt.Errorf("daemon: spawning tab for session %q: %w", name, err) | 312 | + | return domain.UserErr(domain.NoticeTabSpawn, "couldn't open tab: shell failed to start", err) |
| 313 | } | 313 | } | ||
| 314 | tb := newTabWithStableID(tabStableID, paneStableID, pty, tbSize) | 314 | tb := newTabWithStableID(tabStableID, paneStableID, pty, tbSize) | ||
| 315 | if client != nil { | 315 | if client != nil { | ||
M internal/usecase/daemon/session_test.go +24 -0
| @@ -1304,6 +1304,30 @@ func TestRenameSessionUnsafeNameRejected(t *testing.T) { | |||
| 1304 | 1304 | require.ErrorIs(t, d.renameSession(sess, "my work"), domain.ErrInvalidSessionName) | |
| 1305 | 1305 | } | |
| 1306 | 1306 | ||
| 1307 | + | func TestCreateTabPtyFailureIsUserError(t *testing.T) { | |
| 1308 | + | sz := domain.Size{Cols: 80, Rows: 24} | |
| 1309 | + | p, release := newBlockingPTY(t) | |
| 1310 | + | defer release() | |
| 1311 | + | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | |
| 1312 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) | |
| 1313 | + | require.NoError(t, err) | |
| 1314 | + | ||
| 1315 | + | cause := errors.New("fork/exec: no such file") | |
| 1316 | + | failing := portsmocks.NewMockPTYFactory(t) | |
| 1317 | + | failing.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, cause).Once() | |
| 1318 | + | d.ptys = failing | |
| 1319 | + | ||
| 1320 | + | err = d.createTab(sess, sz) | |
| 1321 | + | ||
| 1322 | + | var ue *domain.UserError | |
| 1323 | + | require.ErrorAs(t, err, &ue) | |
| 1324 | + | require.Equal(t, domain.NoticeTabSpawn, ue.Code) | |
| 1325 | + | require.Equal(t, domain.NoticeError, ue.Severity) | |
| 1326 | + | require.Equal(t, "couldn't open tab: shell failed to start", ue.Msg) | |
| 1327 | + | require.ErrorIs(t, err, cause) | |
| 1328 | + | require.NotContains(t, ue.Msg, cause.Error()) | |
| 1329 | + | } | |
| 1330 | + | ||
| 1307 | 1331 | func TestNaturalExitStoppedButExplicitKillPurges(t *testing.T) { | |
| 1308 | 1332 | sz := domain.Size{Cols: 80, Rows: 24} | |
| 1309 | 1333 | p1, release1 := newBlockingPTY(t) | |
| @@ -1304,6 +1304,30 @@ func TestRenameSessionUnsafeNameRejected(t *testing.T) { | |||||
| 1304 | require.ErrorIs(t, d.renameSession(sess, "my work"), domain.ErrInvalidSessionName) | 1304 | require.ErrorIs(t, d.renameSession(sess, "my work"), domain.ErrInvalidSessionName) | ||
| 1305 | } | 1305 | } | ||
| 1306 | 1306 | ||||
| 1307 | + | func TestCreateTabPtyFailureIsUserError(t *testing.T) { | |||
| 1308 | + | sz := domain.Size{Cols: 80, Rows: 24} | |||
| 1309 | + | p, release := newBlockingPTY(t) | |||
| 1310 | + | defer release() | |||
| 1311 | + | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | |||
| 1312 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) | |||
| 1313 | + | require.NoError(t, err) | |||
| 1314 | + | ||||
| 1315 | + | cause := errors.New("fork/exec: no such file") | |||
| 1316 | + | failing := portsmocks.NewMockPTYFactory(t) | |||
| 1317 | + | failing.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, cause).Once() | |||
| 1318 | + | d.ptys = failing | |||
| 1319 | + | ||||
| 1320 | + | err = d.createTab(sess, sz) | |||
| 1321 | + | ||||
| 1322 | + | var ue *domain.UserError | |||
| 1323 | + | require.ErrorAs(t, err, &ue) | |||
| 1324 | + | require.Equal(t, domain.NoticeTabSpawn, ue.Code) | |||
| 1325 | + | require.Equal(t, domain.NoticeError, ue.Severity) | |||
| 1326 | + | require.Equal(t, "couldn't open tab: shell failed to start", ue.Msg) | |||
| 1327 | + | require.ErrorIs(t, err, cause) | |||
| 1328 | + | require.NotContains(t, ue.Msg, cause.Error()) | |||
| 1329 | + | } | |||
| 1330 | + | ||||
| 1307 | func TestNaturalExitStoppedButExplicitKillPurges(t *testing.T) { | 1331 | func TestNaturalExitStoppedButExplicitKillPurges(t *testing.T) { | ||
| 1308 | sz := domain.Size{Cols: 80, Rows: 24} | 1332 | sz := domain.Size{Cols: 80, Rows: 24} | ||
| 1309 | p1, release1 := newBlockingPTY(t) | 1333 | p1, release1 := newBlockingPTY(t) | ||