Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
2 changes: 1 addition & 1 deletion rest/flushchain_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 —
Expand Down
2 changes: 1 addition & 1 deletion rest/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
16 changes: 12 additions & 4 deletions rest/sentry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
//
Expand All @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions rest/sentry_boot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
9 changes: 3 additions & 6 deletions rest/sentry_boot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
22 changes: 10 additions & 12 deletions rest/sentry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -128,41 +128,40 @@ 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", "") })

var enabled bool
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()
require.Len(t, events, 1)
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
Expand All @@ -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 —
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion servers/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading