amadan
instances import: its own stop signals, without SIGPIPE (Plan 6 Task 6 fix round 2)
carlos db's set catches SIGPIPE, which a write to a reset connection raises, so reusing it let a connection reset mid-upload, mid-poll or mid-verify cancel the whole import. The import now uses os.Interrupt, SIGTERM and SIGHUP; the db helper is unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+27
−3
instances_import.go+12 −3instances_import_test.go+15 −0
CI passed — run details
diff --git a/instances_import.go b/instances_import.go| index aa2bba149..f5573189d 100644 |
| --- a/instances_import.go |
| +++ b/instances_import.go |
| @@ -12,9 +12,11 @@ import ( |
| "net/http" |
| "net/url" |
| "os" |
| + "os/signal" |
| "slices" |
| "strconv" |
| "strings" |
| + "syscall" |
| "time" |
| ) |
| @@ -50,6 +52,14 @@ type importLive struct{ lines []string } |
| func (e importLive) Error() string { return strings.Join(e.lines, "\n") } |
| +// importStopSignals end an import early, through its live exit: Ctrl-C, a |
| +// supervisor's SIGTERM, and a closed terminal (SIGHUP). Not SIGPIPE, unlike |
| +// carlos db's set: a write to a reset connection (the upload, a poll, the |
| +// verify) raises it, and that must stay the request's own error rather than |
| +// cancel the whole import. syscall.SIGHUP and SIGTERM exist on every |
| +// platform the CLI builds for, as carlos db's set relies on. |
| +var importStopSignals = []os.Signal{os.Interrupt, syscall.SIGTERM, syscall.SIGHUP} |
| + |
| // importJob is one `carlos instances import`. |
| type importJob struct { |
| acct apiAcct |
| @@ -98,9 +108,8 @@ func runInstancesImport(args []string) { |
| } |
| // Ctrl-C, SIGTERM or a closed terminal cancels the import's context |
| // rather than killing the process, so an import left live still shows |
| - // its token and the commands to finish it (M17). The signals are |
| - // carlos db's. |
| - ctx, stop := dbSignalContext() |
| + // its token and the commands to finish it (M17). |
| + ctx, stop := signal.NotifyContext(context.Background(), importStopSignals...) |
| defer stop() |
| err := sess.runImport(ctx, stdout, importJob{acct: acct, app: app2, host: strings.TrimSpace(*host), file: *file, |
| replaces: strings.TrimSpace(*replaces), environment: strings.TrimSpace(*environment), verify: strings.TrimSpace(*verify), message: message}) |
diff --git a/instances_import_test.go b/instances_import_test.go| index 2800ce51b..8ecb61bd5 100644 |
| --- a/instances_import_test.go |
| +++ b/instances_import_test.go |
| @@ -15,6 +15,7 @@ import ( |
| "strings" |
| "sync" |
| "sync/atomic" |
| + "syscall" |
| "testing" |
| "time" |
| ) |
| @@ -863,3 +864,17 @@ func TestImportVerifyTarget(t *testing.T) { |
| t.Fatal("the request would have been made") |
| } |
| } |
| + |
| +// The import's own signal set: Ctrl-C, SIGTERM and SIGHUP cancel it, and |
| +// SIGPIPE never does, since a reset connection raises it mid-upload. |
| +func TestImportStopSignalsLeaveOutSIGPIPE(t *testing.T) { |
| + want := map[os.Signal]bool{os.Interrupt: true, syscall.SIGTERM: true, syscall.SIGHUP: true} |
| + if len(importStopSignals) != len(want) { |
| + t.Fatalf("importStopSignals = %v", importStopSignals) |
| + } |
| + for _, sig := range importStopSignals { |
| + if sig == syscall.SIGPIPE || !want[sig] { |
| + t.Fatalf("importStopSignals = %v, want exactly Interrupt, SIGTERM and SIGHUP", importStopSignals) |
| + } |
| + } |
| +} |