Skip to content

feat(api): thread context.Context through the client; cancel cleanly on SIGINT - #48

Merged
kshahbw merged 1 commit into
Bandwidth:mainfrom
kush2shah:fix/context-cancellation
Aug 25, 2026
Merged

feat(api): thread context.Context through the client; cancel cleanly on SIGINT#48
kshahbw merged 1 commit into
Bandwidth:mainfrom
kush2shah:fix/context-cancellation

Conversation

@kush2shah

Copy link
Copy Markdown
Contributor

Fixes #25. Fixes #34.

What this does

One PR for both issues because #34's complete fix requires #25's plumbing — a signal handler without request contexts only cancels between polls, not during one.

#34 — signal handling. Execute() now runs the command tree under signal.NotifyContext(SIGINT, SIGTERM) via rootCmd.ExecuteContext. The first Ctrl-C cancels cmd.Context() everywhere: the in-flight HTTP request aborts, --wait loops exit through their cancellation branches, and tendlc's awaitTerminal emits the receipt with bandwidthId before the process exits — the accepted-but-unprinted brand ID from the issue's reproduction is no longer lost. A second Ctrl-C is untrapped (NotifyContext stops after one signal) and hard-exits the Go-default way.

#25 — context threading. Requester and every api.Client method take ctx context.Context as the first parameter, and requests are built with http.NewRequestWithContext. On the open design question, this changes the signatures in place rather than adding parallel …Ctx methods, as the issue leans: one mechanical diff, no lingering dual API, pre-1.0. All ~150 call sites migrated: commands pass cmd.Context(); the internal/sip, internal/tendlc, and internal/customerprofile service layers thread ctx through their methods; --wait helper functions take ctx; the testutil.FakeClient and test fakes match the new interface.

Poll wiring. Every cmdutil.Poll site (13 of them) now sets PollConfig.Context, so the Context field added in the 10DLC foundation work finally has a producer.

Scope note

auth.TokenManager.GetToken (token exchange) still runs without a context — it has its own timeout, and that path is entangled with the #24 auth-status work, so it's deliberately left out.

Tests

  • TestClient_ContextCancelsInFlightRequest — a context deadline aborts an in-flight HTTP request against a deliberately-hanging server (fails against the old http.NewRequest build).
  • TestExecuteCancelsCommandContextOnSIGINT — sends a real SIGINT to the test process and asserts the command's context cancels and Execute returns (//go:build !windows). The issue's "second Ctrl-C hard-exits" behavior comes from NotifyContext semantics and can't be asserted in-process.
  • Existing suite: go build, go vet, gofmt clean; full tests pass; internal/api, internal/cmdutil, and cmd race-clean over -count=3.

Docs

AGENTS.md's two interim notes — the untrappable-Ctrl-C caveat on the receipt guarantee (brands and campaigns) — are replaced with the new guarantee: a single Ctrl-C emits the receipt; only a second one kills the process without output.

Execute() now runs the command tree under signal.NotifyContext, so the
first SIGINT/SIGTERM cancels cmd.Context() instead of killing the
process. Requester and every api.Client method take a context as their
first parameter, requests are built with NewRequestWithContext, and all
~150 call sites pass cmd.Context() (or a threaded ctx) through commands,
the sip/tendlc/customerprofile service layers, and the --wait helpers.
Every cmdutil.Poll site now sets PollConfig.Context, so the cancellation
plumbing added in the 10DLC foundation work finally has a producer:
Ctrl-C during --wait aborts the in-flight request and exits through the
receipt-emitting paths — tendlc's awaitTerminal prints the accepted
resource's bandwidthId instead of dying with nothing on stdout. A second
Ctrl-C is untrapped (NotifyContext stops after one signal) and
hard-exits.

Requester's signatures changed in place rather than growing parallel
...Ctx methods — the call sites are mechanical and this is pre-1.0, per
the discussion in Bandwidth#25. Token exchange (auth.TokenManager.GetToken) still
runs without a context; that path has its own timeout and is left for
the auth-status work.

New tests: a client test proving a context deadline aborts an in-flight
HTTP request, and a cmd test proving a real SIGINT delivered to the
process cancels the command context and Execute returns. AGENTS.md's
interim notes about the untrappable Ctrl-C are replaced with the new
guarantee.

Fixes Bandwidth#25. Fixes Bandwidth#34.
@kush2shah
kush2shah requested review from a team as code owners August 25, 2026 15:55
@bwappsec

bwappsec commented Aug 25, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@kshahbw
kshahbw merged commit ece0720 into Bandwidth:main Aug 25, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants