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/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 7aa8040..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() == nil { + if !sentryCaptureEnabled() { next.ServeHTTP(w, r) return } diff --git a/rest/sentry_boot.go b/rest/sentry_boot.go index 0545eb5..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() == nil { + 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 6eaeca9..d7c2236 100644 --- a/rest/sentry_boot_test.go +++ b/rest/sentry_boot_test.go @@ -131,21 +131,18 @@ 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) { - require.Nil(t, sentry.CurrentHub().Client(), "test requires the disabled state") - +func TestFlushSentryOnShutdown_CaptureDisabled_WarnsAndSkips(t *testing.T) { 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..adb521d 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,18 +128,18 @@ 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. -func TestSetupSentry_InvalidDSNFailsSafeWithoutClient(t *testing.T) { +// disabled, the hub's client stays disabled, and nothing panics. The API then +// runs exactly as in the no-DSN case. +func TestSetupSentry_InvalidDSNFailsSafeWithCaptureDisabled(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() @@ -162,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 @@ -175,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 — @@ -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) { 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