diff --git a/.changeset/sdk-effect-5xx-response-failure.md b/.changeset/sdk-effect-5xx-response-failure.md new file mode 100644 index 0000000000..f589ed3d9e --- /dev/null +++ b/.changeset/sdk-effect-5xx-response-failure.md @@ -0,0 +1,5 @@ +--- +"@maple-dev/effect-sdk": patch +--- + +A 5xx server span keeps its `HttpServerErrorResponse` exception on Effect 4.0.2 and later, which now fails the span itself with an `HttpServerError` for any 5xx response. Without this, those responses were grouped as one generic `HttpServerError` issue. diff --git a/apps/ai/src/worker/http.test.ts b/apps/ai/src/worker/http.test.ts index 6285183e67..e86579f87f 100644 --- a/apps/ai/src/worker/http.test.ts +++ b/apps/ai/src/worker/http.test.ts @@ -17,11 +17,14 @@ import { makeFetch } from "./http" */ const ExportedSpan = Schema.Struct({ name: Schema.String, + kind: Schema.optionalKey(Schema.Finite), attributes: Schema.Array( Schema.Struct({ key: Schema.String, value: Schema.Record(Schema.String, Schema.Unknown) }), ), }) type ExportedSpan = typeof ExportedSpan.Type +/** OTLP `SPAN_KIND_SERVER`. Effect names server spans by method alone, so kind is what marks one. */ +const SERVER_KIND = 2 const decodeExportedTraces = Schema.decodeUnknownSync( Schema.Struct({ resourceSpans: Schema.optionalKey( @@ -59,7 +62,7 @@ const capturedServerSpans = Effect.acquireRelease( decodeExportedTraces(JSON.parse(String(init?.body ?? "{}"))).resourceSpans ?? [], ) .flatMap((resource) => resource.scopeSpans.flatMap((scope) => scope.spans)) - .filter((span) => span.name.startsWith("http.server ")), + .filter((span) => span.kind === SERVER_KIND), ), ) @@ -126,21 +129,21 @@ describe("the maple-ai Worker through alchemy's bridge", () => { assert.deepStrictEqual(health, { status: 200, - span: "http.server GET", + span: "GET", ordinal: undefined, ageMs: undefined, headers: { ordinal: undefined, ageMs: undefined }, }) assert.deepStrictEqual(first, { status: 200, - span: "http.server POST", + span: "POST", ordinal: "1", ageMs: "0", headers: { ordinal: "1", ageMs: "0" }, }) assert.deepStrictEqual(failed, { status: 503, - span: "http.server POST", + span: "POST", ordinal: "2", ageMs: "250", headers: { ordinal: "2", ageMs: "250" }, diff --git a/apps/api/src/worker-bridge.test.ts b/apps/api/src/worker-bridge.test.ts index 23f1f3ae2d..87bac731cf 100644 --- a/apps/api/src/worker-bridge.test.ts +++ b/apps/api/src/worker-bridge.test.ts @@ -29,6 +29,7 @@ const ExportedAttribute = Schema.Struct({ }) const ExportedSpan = Schema.Struct({ name: Schema.String, + kind: Schema.optionalKey(Schema.Finite), status: Schema.Struct({ code: Schema.optionalKey(Schema.Finite) }), attributes: Schema.Array(ExportedAttribute), events: Schema.optionalKey( @@ -36,6 +37,8 @@ const ExportedSpan = Schema.Struct({ ), }) type ExportedSpan = typeof ExportedSpan.Type +/** OTLP `SPAN_KIND_SERVER`. Effect names server spans by method alone, so kind is what marks one. */ +const SERVER_KIND = 2 const ExportedTraces = Schema.Struct({ resourceSpans: Schema.optionalKey( Schema.Array( @@ -79,7 +82,7 @@ const serverSpans = (recorded: ReadonlyArray): Array request.url.endsWith("/v1/traces")) .flatMap((request) => decodeExportedTraces(JSON.parse(request.body ?? "{}")).resourceSpans ?? []) .flatMap((resource) => resource.scopeSpans.flatMap((scope) => scope.spans)) - .filter((span) => span.name.startsWith("http.server ")) + .filter((span) => span.kind === SERVER_KIND) const logBodies = (recorded: ReadonlyArray): Array => recorded diff --git a/apps/electric-sync/src/worker-bridge.test.ts b/apps/electric-sync/src/worker-bridge.test.ts index 72ce8b8adb..e693221e6d 100644 --- a/apps/electric-sync/src/worker-bridge.test.ts +++ b/apps/electric-sync/src/worker-bridge.test.ts @@ -22,6 +22,7 @@ import { */ const ExportedSpan = Schema.Struct({ name: Schema.String, + kind: Schema.optionalKey(Schema.Finite), status: Schema.Struct({ code: Schema.optionalKey(Schema.Finite), message: Schema.optionalKey(Schema.String), @@ -31,6 +32,8 @@ const ExportedSpan = Schema.Struct({ ), }) type ExportedSpan = typeof ExportedSpan.Type +/** OTLP `SPAN_KIND_SERVER`. Effect names server spans by method alone, so kind is what marks one. */ +const SERVER_KIND = 2 const ExportedTraces = Schema.Struct({ resourceSpans: Schema.optionalKey( @@ -106,7 +109,7 @@ const event = (path: string, app: ReturnType = appLayer()) => return { status: response.status, flushedBeforeClose, - server: spans.filter((span) => span.name === "http.server GET"), + server: spans.filter((span) => span.kind === SERVER_KIND), } }).pipe(Effect.scoped) diff --git a/apps/web/src/components/agent-sessions/session-detail/session-transcript.tsx b/apps/web/src/components/agent-sessions/session-detail/session-transcript.tsx index 2077dd0a08..2f263dea7a 100644 --- a/apps/web/src/components/agent-sessions/session-detail/session-transcript.tsx +++ b/apps/web/src/components/agent-sessions/session-detail/session-transcript.tsx @@ -650,6 +650,7 @@ function SystemBlock({ rendered={body.rendered} raw={raw} onRawChange={(next) => next !== raw && onToggleRow(rawKey)} + className="self-start" /> {/* Copies the prompt as captured, not its rendering. */} diff --git a/apps/web/src/components/agent-sessions/session-detail/span-expansion.tsx b/apps/web/src/components/agent-sessions/session-detail/span-expansion.tsx index b76bcd0d20..5c41ac401f 100644 --- a/apps/web/src/components/agent-sessions/session-detail/span-expansion.tsx +++ b/apps/web/src/components/agent-sessions/session-detail/span-expansion.tsx @@ -318,7 +318,12 @@ function SystemMessageRow({ message }: { message: SpanMessage }) { proseClassName="text-sm" /> - + {/* Copies the instructions as captured, not their rendering. */} diff --git a/packages/backend/src/services/warehouse/WarehouseQueryService.test.ts b/packages/backend/src/services/warehouse/WarehouseQueryService.test.ts index a30df8632d..8337473c1c 100644 --- a/packages/backend/src/services/warehouse/WarehouseQueryService.test.ts +++ b/packages/backend/src/services/warehouse/WarehouseQueryService.test.ts @@ -1388,6 +1388,9 @@ describe("warehouse spans follow the database conventions", () => { return { spans, tracer } } const okFetch: typeof fetch = async () => new Response('{"data":[]}') + // By kind and method, not name: Effect names it by method alone, and the database span is a client too. + const isHttpClientSpan = (span: Tracer.NativeSpan) => + span.kind === "client" && span.attributes.has("http.request.method") it.effect("emits one database span per query and no http.client span beneath it", () => { const layer = buildLayer(createTestDb(trackedDbs)) @@ -1398,9 +1401,11 @@ describe("warehouse spans follow the database conventions", () => { context: "spanShape", }), ).pipe(Effect.withTracer(tracer)) - const names = spans.map((span) => span.name) - assert.include(names, "WarehouseQueryService.executeSql") - assert.isFalse(names.some((name) => name.startsWith("http.client"))) + assert.include( + spans.map((span) => span.name), + "WarehouseQueryService.executeSql", + ) + assert.isFalse(spans.some(isHttpClientSpan)) }).pipe(Effect.provide(layer), Effect.provideService(FetchHttpClient.Fetch, okFetch)) }) @@ -1414,7 +1419,7 @@ describe("warehouse spans follow the database conventions", () => { okFetch, ) yield* client.sql(parseStatement("SELECT 1 FORMAT JSON")).pipe(Effect.withTracer(tracer)) - assert.isTrue(spans.some((span) => span.name === "http.client POST")) + assert.isTrue(spans.some(isHttpClientSpan)) }), ) }) diff --git a/packages/effect-sdk/src/shared/flushable-tracer.test.ts b/packages/effect-sdk/src/shared/flushable-tracer.test.ts index bc87be854c..d575bd4110 100644 --- a/packages/effect-sdk/src/shared/flushable-tracer.test.ts +++ b/packages/effect-sdk/src/shared/flushable-tracer.test.ts @@ -4,6 +4,7 @@ import * as ErrorReporter from "effect/ErrorReporter" import { HttpApiSchemaError } from "effect/http-api/HttpApiError" import * as HttpServerError from "effect/http/HttpServerError" import * as HttpServerRequest from "effect/http/HttpServerRequest" +import * as HttpServerResponse from "effect/http/HttpServerResponse" import { makeSpanBuffer } from "./flushable-tracer.js" // A benign error flagged exactly the way Effect's RouteNotFound is. @@ -254,6 +255,37 @@ describe("makeSpanBuffer rendered 5xx responses", () => { }), ) + it.effect("treats Effect's own failure for a 5xx response as the rendered 5xx", () => + Effect.gen(function* () { + const buffer = makeSpanBuffer() + const request = HttpServerRequest.fromWeb(new Request("http://localhost/api/sync/shape")) + const error = new HttpServerError.HttpServerError({ + reason: new HttpServerError.ResponseError({ + request, + response: HttpServerResponse.empty({ status: 500 }), + }), + }) + yield* Effect.annotateCurrentSpan({ + "http.request.method": "GET", + "url.path": "/api/sync/shape", + "http.response.status_code": 500, + }).pipe( + Effect.andThen(Effect.fail(error)), + Effect.withSpan("GET", { kind: "server" }), + Effect.provide(buffer.tracerLayer), + Effect.ignore, + ) + const [span] = buffer.drain() + assert.strictEqual(span!.status.message, "HTTP 500 (GET /api/sync/shape)") + const exceptions = span!.events.filter((event) => event.name === "exception") + assert.strictEqual(exceptions.length, 1) + assert.deepStrictEqual( + exceptions[0]!.attributes.find((attribute) => attribute.key === "exception.type")?.value, + { stringValue: "HttpServerErrorResponse" }, + ) + }), + ) + it.effect("keeps an exception the handler recorded itself instead of relabelling it", () => Effect.gen(function* () { const buffer = makeSpanBuffer() diff --git a/packages/effect-sdk/src/shared/flushable-tracer.ts b/packages/effect-sdk/src/shared/flushable-tracer.ts index d84faade14..371863d706 100644 --- a/packages/effect-sdk/src/shared/flushable-tracer.ts +++ b/packages/effect-sdk/src/shared/flushable-tracer.ts @@ -258,6 +258,21 @@ const renderedServerError = (self: SpanImpl): number | undefined => { return Number.isInteger(code) && code >= 500 ? code : undefined } +// Effect 4.0.2+ fails a 5xx server span itself, with an `HttpServerError` whose reason is +// `ResponseError`. That names nothing the rendered 5xx did not, so it takes the same road. +const isResponseFailure = (exit: Exit.Exit): boolean => { + if (exit._tag === "Success") return true + const [reason, ...rest] = exit.cause.reasons + return ( + rest.length === 0 && + reason !== undefined && + Cause.isFailReason(reason) && + Predicate.isTagged(reason.error, "HttpServerError") && + Predicate.hasProperty(reason.error, "reason") && + Predicate.isTagged(reason.error.reason, "ResponseError") + ) +} + const makeOtlpSpan = (self: SpanImpl, anticipatedErrorIdentifiers?: ReadonlySet): OtlpSpan => { const status = self.status as ExtractTag const attributes = OtlpResource.entriesToAttributes(self.attributes.entries()) @@ -269,7 +284,7 @@ const makeOtlpSpan = (self: SpanImpl, anticipatedErrorIdentifiers?: ReadonlySet< })) let otelStatus: Status - const serverError = status.exit._tag === "Success" ? renderedServerError(self) : undefined + const serverError = isResponseFailure(status.exit) ? renderedServerError(self) : undefined if (serverError !== undefined) { const method = self.attributes.get("http.request.method") const path = self.attributes.get("url.path")