From af2d9dd624aad2c4bc1f98a7a6048d576ad39dbb Mon Sep 17 00:00:00 2001 From: SyniRon <66834451+SyniRon@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:39:26 -0400 Subject: [PATCH 1/2] build(deps): bump sentry-go to 0.50.0 and gate on IsEnabled sentry-go 0.50.0 binds a disabled no-op client where it used to leave nil, so the two "Sentry is off" guards in rest compared against nil and never fired. Without a DSN the middleware recovered panics into 500s and FlushSentryOnShutdown installed its signal handler. Both guards now ask Client().IsEnabled(). The tests stop asserting a nil client. SetupSentry's no-DSN and bad-DSN tests check IsEnabled() instead. Preconditions that only duplicated the behavior assertions are gone, and the flush-guard test checks that a warning was logged, not its wording. Refs #271 > *This was generated by AI* --- go.mod | 2 +- go.sum | 4 ++-- rest/sentry.go | 2 +- rest/sentry_boot.go | 2 +- rest/sentry_boot_test.go | 5 +---- rest/sentry_test.go | 16 +++++++--------- 6 files changed, 13 insertions(+), 18 deletions(-) diff --git a/go.mod b/go.mod index d81eaad..81aa5b7 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/7cav/api go 1.26.0 require ( - github.com/getsentry/sentry-go v0.49.0 + github.com/getsentry/sentry-go v0.50.0 github.com/go-sql-driver/mysql v1.10.1 github.com/pb33f/libopenapi v0.41.5 github.com/pb33f/libopenapi-validator v0.16.0 diff --git a/go.sum b/go.sum index 5541605..153e5a9 100644 --- a/go.sum +++ b/go.sum @@ -21,8 +21,8 @@ github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHk github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S9k= github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= -github.com/getsentry/sentry-go v0.49.0 h1:Ehejknu1l023Ub7QoRBVLAI7g3Jnhqku4oWx4B4Sh5s= -github.com/getsentry/sentry-go v0.49.0/go.mod h1:nuMJAoCfe1u0Bts2ocyNI+TW8HT84vRMqwA5Qq/SKUI= +github.com/getsentry/sentry-go v0.50.0 h1:gkO59rn/i0RCAXUUTflC5QVbTQGPXbaFabs2olVzc9g= +github.com/getsentry/sentry-go v0.50.0/go.mod h1:+xsDuN+SoRN/xfs1hdq78wnrE3omIZf+FCuh9FOW1C8= github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA= github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og= github.com/go-openapi/jsonpointer v1.0.2 h1:NbW7xT2b+l7x+JefiPGDsZWsrLk1ogrz4AHK1TTzMt8= diff --git a/rest/sentry.go b/rest/sentry.go index 7aa8040..422da68 100644 --- a/rest/sentry.go +++ b/rest/sentry.go @@ -171,7 +171,7 @@ func sentryLabelsFromContext(ctx context.Context) *sentryLabels { // and the client sees the truncation instead of trusting a half response. func sentryMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if sentry.CurrentHub().Client() == nil { + if !sentry.CurrentHub().Client().IsEnabled() { next.ServeHTTP(w, r) return } diff --git a/rest/sentry_boot.go b/rest/sentry_boot.go index 0545eb5..05ebdfe 100644 --- a/rest/sentry_boot.go +++ b/rest/sentry_boot.go @@ -62,7 +62,7 @@ func sentryDialCheck(rawDSN string) { // the call site, because installing the handler without a client would // silently rewrite the process's signal semantics for nothing. func FlushSentryOnShutdown() { - if sentry.CurrentHub().Client() == nil { + if !sentry.CurrentHub().Client().IsEnabled() { // Only reachable on programmer error — the call site gates on // SetupSentry. Loud so misuse never silently skips the handler. Warn.Println("FlushSentryOnShutdown called without an initialised sentry client — shutdown flush handler not installed") diff --git a/rest/sentry_boot_test.go b/rest/sentry_boot_test.go index 6eaeca9..bd8854f 100644 --- a/rest/sentry_boot_test.go +++ b/rest/sentry_boot_test.go @@ -136,16 +136,13 @@ func TestSentryDebugEnabled_StrictParse(t *testing.T) { // SetupSentry), and silent misuse would mean shutdown flushes everyone assumes // exist were never installed. func TestFlushSentryOnShutdown_NoClient_WarnsAndSkips(t *testing.T) { - require.Nil(t, sentry.CurrentHub().Client(), "test requires the disabled state") - var warnBuf bytes.Buffer Warn.SetOutput(&warnBuf) defer Warn.SetOutput(os.Stdout) FlushSentryOnShutdown() - assert.Contains(t, warnBuf.String(), "without an initialised sentry client", - "misuse must be visible in logs, not a silent no-op") + assert.NotEmpty(t, warnBuf.String(), "misuse must be visible in logs, not a silent no-op") } func TestWatchShutdown_FlushesBeforeExit(t *testing.T) { diff --git a/rest/sentry_test.go b/rest/sentry_test.go index 2794fcb..2c16c7c 100644 --- a/rest/sentry_test.go +++ b/rest/sentry_test.go @@ -112,7 +112,7 @@ var errOutageSentry = errors.New("simulated datastore outage") // enableSentry drives the PRODUCTION init path (SetupSentry) with the // transport seam swapped for a capture mock, and restores the disabled state -// afterwards so the rest of the package's tests keep running with no client. +// afterwards so the rest of the package's tests keep running with capture off. func enableSentry(t *testing.T) *transportMock { t.Helper() tr := &transportMock{} @@ -128,17 +128,17 @@ func enableSentry(t *testing.T) *transportMock { } // No DSN → complete no-op: nothing is initialised, exactly as in Phase 0 — -// no client, so every capture point in the package stays a pass-through. +// capture is off, so every capture point in the package stays a pass-through. func TestSetupSentry_NoDSNInitialisesNothing(t *testing.T) { viper.Set("SENTRY_DSN", "") assert.False(t, SetupSentry(testRelease), "no DSN must report capture disabled") - assert.Nil(t, sentry.CurrentHub().Client(), "no DSN must bind no client") + assert.False(t, sentry.CurrentHub().Client().IsEnabled(), "no DSN must leave capture disabled") } // A malformed DSN must fail SAFE — telemetry must never take the API down // (the init-failure branch in SetupSentry): Init errors, capture reports -// disabled, no client binds, and nothing panics. The API then runs exactly as -// in the no-DSN case. +// disabled, the hub's client stays disabled, and nothing panics. The API then +// runs exactly as in the no-DSN case. func TestSetupSentry_InvalidDSNFailsSafeWithoutClient(t *testing.T) { viper.Set("SENTRY_DSN", "not-a-dsn") t.Cleanup(func() { viper.Set("SENTRY_DSN", "") }) @@ -147,14 +147,13 @@ func TestSetupSentry_InvalidDSNFailsSafeWithoutClient(t *testing.T) { require.NotPanics(t, func() { enabled = SetupSentry(testRelease) }, "a bad DSN must never panic the boot path") assert.False(t, enabled, "a failed init must report capture disabled") - assert.Nil(t, sentry.CurrentHub().Client(), "a failed init must bind no client") + assert.False(t, sentry.CurrentHub().Client().IsEnabled(), "a failed init must leave capture disabled") } // With a DSN the client is initialised and release-tagged from the build-time // version the caller passes — every event carries it. func TestSetupSentry_DSNEnablesClientWithRelease(t *testing.T) { tr := enableSentry(t) - require.NotNil(t, sentry.CurrentHub().Client(), "a DSN must bind a client") sentry.CaptureMessage("wiring probe") events := tr.Events() @@ -381,13 +380,12 @@ func TestSentry_BeforeSendScrubsRequestAuthMaterial(t *testing.T) { assert.Equal(t, "sweep-test", ev.Request.Headers["User-Agent"], "non-auth headers survive") } -// No DSN → complete no-op (Phase 0 parity): with no client bound the sentry +// No DSN → complete no-op (Phase 0 parity): with capture disabled the sentry // layer is a pass-through — panics propagate exactly as they do today // (net/http recovers them per-connection; here, to the test) with nothing // written, and 5xx responses flow through the choke point unchanged with no // capture machinery in the path. func TestSentry_NoDSNIsCompletePassThrough(t *testing.T) { - require.Nil(t, sentry.CurrentHub().Client(), "precondition: no client bound") captureErrorLog(t) h := New(&sentryFakeDatastore{findAllRanks: func() ([]*types.RankExpanded, error) { From 532969542f5327dd56ac9d63646d6123ded9a81b Mon Sep 17 00:00:00 2001 From: SyniRon <66834451+SyniRon@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:42:35 -0400 Subject: [PATCH 2/2] refactor(sentry): one capture-enabled check, drop "no client" wording Both guards now call sentryCaptureEnabled(), so the next SDK change to what "off" looks like lands in one place. Two test names and the comments that said "no client" or "client bound" now say capture is off or on, since 0.50.0 always binds a client. Refs #271 > *This was generated by AI* --- rest/flushchain_internal_test.go | 2 +- rest/rest.go | 2 +- rest/sentry.go | 16 ++++++++++++---- rest/sentry_boot.go | 4 ++-- rest/sentry_boot_test.go | 4 ++-- rest/sentry_test.go | 6 +++--- servers/server.go | 2 +- 7 files changed, 22 insertions(+), 14 deletions(-) diff --git a/rest/flushchain_internal_test.go b/rest/flushchain_internal_test.go index e630600..14aed5e 100644 --- a/rest/flushchain_internal_test.go +++ b/rest/flushchain_internal_test.go @@ -30,7 +30,7 @@ import ( ) func TestFlushChain_NewShapedStackStreamsFlushedPrefix(t *testing.T) { - // Sentry ENABLED: without a client sentryMiddleware never mounts + // Sentry ENABLED: with capture off sentryMiddleware never mounts // commitWriter, and the composed property would silently exclude the // outermost wrapper. The premise that an enabled client mounts the // wrapper UNCONDITIONALLY is itself pinned — diff --git a/rest/rest.go b/rest/rest.go index 8a992ef..7dd2d1b 100644 --- a/rest/rest.go +++ b/rest/rest.go @@ -20,7 +20,7 @@ // - sentryMiddleware (#132, sentry.go): panic recovery at the front of the // chain — catches the metrics layer's re-panic, reports the event, and // writes the contract 500; 5xx Sentry reports hook the writeError choke -// point. Env-gated by SENTRY_DSN (rest.SetupSentry): without a client it +// point. Env-gated by SENTRY_DSN (rest.SetupSentry): with capture off it // is a complete no-op. Its route/key-id tags reach this OUTER layer via // the sentryLabels context holder, filled by the sentryLabel wrapper // inside auth (the same mechanism as metricLabels below). diff --git a/rest/sentry.go b/rest/sentry.go index 422da68..e0c15fa 100644 --- a/rest/sentry.go +++ b/rest/sentry.go @@ -55,8 +55,8 @@ var sentryTransport sentry.Transport // SetupSentry initialises Sentry error capture (errors only, no tracing) when // SENTRY_DSN is present in the environment — the same gate as Phase 0's // servers.setupSentry, which this replaces at cutover (#134). Without a DSN -// nothing is initialised — no client, no capture, local/dev unaffected; the -// only side effect is one Info line making the disabled state visible at +// nothing is initialised and capture stays off, so local/dev is unaffected. +// The only side effect is one Info line making the disabled state visible at // boot. Returns whether capture was enabled. // // release is the build-time version (the -X servers.version injection today; @@ -108,6 +108,14 @@ func SetupSentry(release string) bool { return true } +// sentryCaptureEnabled reports whether the current hub's client captures +// events, which is true only after SetupSentry initialised one. Since +// sentry-go 0.50.0 the SDK binds a disabled no-op client by default, so +// checking for a nil client no longer detects "off". +func sentryCaptureEnabled() bool { + return sentry.CurrentHub().Client().IsEnabled() +} + // sentryDebugEnabled reads SENTRY_DEBUG strictly (mirrors Phase 0, which this // file replaces at cutover). viper.GetBool silently maps unparseable values // ("yes", "on", typos) to false — a mid-incident operator trap: debug looks @@ -160,7 +168,7 @@ func sentryLabelsFromContext(ctx context.Context) *sentryLabels { // sentryLabel filled on the way out), and writes the contract 500 error shape // so the request still completes. // -// Without an initialised client (no SENTRY_DSN) it is a complete no-op: +// With capture disabled (no SENTRY_DSN) it is a complete no-op: // requests flow unchanged and panics propagate exactly as they do today // (net/http recovers them per-connection) — Phase 0 parity. // @@ -171,7 +179,7 @@ func sentryLabelsFromContext(ctx context.Context) *sentryLabels { // and the client sees the truncation instead of trusting a half response. func sentryMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if !sentry.CurrentHub().Client().IsEnabled() { + if !sentryCaptureEnabled() { next.ServeHTTP(w, r) return } diff --git a/rest/sentry_boot.go b/rest/sentry_boot.go index 05ebdfe..9bf928d 100644 --- a/rest/sentry_boot.go +++ b/rest/sentry_boot.go @@ -59,10 +59,10 @@ func sentryDialCheck(rawDSN string) { // minimal hook so error events from the final moments are not lost. The // composition root calls it only when SetupSentry returned true, so the no-DSN // path keeps the default signal behavior exactly — guarded here as well as at -// the call site, because installing the handler without a client would +// the call site, because installing the handler with capture disabled would // silently rewrite the process's signal semantics for nothing. func FlushSentryOnShutdown() { - if !sentry.CurrentHub().Client().IsEnabled() { + if !sentryCaptureEnabled() { // Only reachable on programmer error — the call site gates on // SetupSentry. Loud so misuse never silently skips the handler. Warn.Println("FlushSentryOnShutdown called without an initialised sentry client — shutdown flush handler not installed") diff --git a/rest/sentry_boot_test.go b/rest/sentry_boot_test.go index bd8854f..d7c2236 100644 --- a/rest/sentry_boot_test.go +++ b/rest/sentry_boot_test.go @@ -131,11 +131,11 @@ func TestSentryDebugEnabled_StrictParse(t *testing.T) { } } -// TestFlushSentryOnShutdown_NoClient_WarnsAndSkips pins the guard's +// TestFlushSentryOnShutdown_CaptureDisabled_WarnsAndSkips pins the guard's // visibility: it only fires on programmer error (the call site must gate on // SetupSentry), and silent misuse would mean shutdown flushes everyone assumes // exist were never installed. -func TestFlushSentryOnShutdown_NoClient_WarnsAndSkips(t *testing.T) { +func TestFlushSentryOnShutdown_CaptureDisabled_WarnsAndSkips(t *testing.T) { var warnBuf bytes.Buffer Warn.SetOutput(&warnBuf) defer Warn.SetOutput(os.Stdout) diff --git a/rest/sentry_test.go b/rest/sentry_test.go index 2c16c7c..adb521d 100644 --- a/rest/sentry_test.go +++ b/rest/sentry_test.go @@ -139,7 +139,7 @@ func TestSetupSentry_NoDSNInitialisesNothing(t *testing.T) { // (the init-failure branch in SetupSentry): Init errors, capture reports // disabled, the hub's client stays disabled, and nothing panics. The API then // runs exactly as in the no-DSN case. -func TestSetupSentry_InvalidDSNFailsSafeWithoutClient(t *testing.T) { +func TestSetupSentry_InvalidDSNFailsSafeWithCaptureDisabled(t *testing.T) { viper.Set("SENTRY_DSN", "not-a-dsn") t.Cleanup(func() { viper.Set("SENTRY_DSN", "") }) @@ -161,7 +161,7 @@ func TestSetupSentry_DSNEnablesClientWithRelease(t *testing.T) { assert.Equal(t, testRelease, events[0].Release, "events must carry the build-time release") } -// commitWriter mounts UNCONDITIONALLY whenever a client is bound — the +// commitWriter mounts UNCONDITIONALLY whenever capture is enabled — the // composed flush-chain pin (flushchain_internal_test.go) enables sentry on // exactly this premise, so a refactor that mounts the wrapper conditionally // (per-route, per-method, …) fails here by name instead of silently @@ -174,7 +174,7 @@ func TestSentryMiddleware_MountsCommitWriterWhenEnabled(t *testing.T) { })) h.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/", nil)) require.True(t, sawCommitWriter, - "with a client bound, sentryMiddleware must hand every handler a *commitWriter — the recovery's committed signal and the composed flush-chain pin both stand on it") + "with capture enabled, sentryMiddleware must hand every handler a *commitWriter — the recovery's committed signal and the composed flush-chain pin both stand on it") } // The headline acceptance (#132): a panicking handler produces ONE event — diff --git a/servers/server.go b/servers/server.go index b38b598..4cf3930 100644 --- a/servers/server.go +++ b/servers/server.go @@ -147,7 +147,7 @@ func (server *MicroServer) Start() { } // Observability (PRD #112): errors-only Sentry capture, gated on SENTRY_DSN. - // Disabled (local/dev) nothing is initialised: one Info line, no client, no + // Disabled (local/dev) nothing is initialised: one Info line, no capture, no // signal handler, so shutdown behaves exactly as before. Enabled, the dial // pre-check BLOCKS boot before any listener opens, for up to 3s on an // unreachable host. That delay is accepted by design so the log names a