diff --git a/.oxlintrc.json b/.oxlintrc.json index a75a2abfb..8bed80e10 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -38,7 +38,9 @@ "maple/no-ordie-compiled-query": "error", "maple/no-react-use-effect": "warn", "maple/no-record-string-any": "error", + "maple/no-try-catch": "error", "typescript/no-explicit-any": "warn", + "typescript/no-non-null-assertion": "error", "no-alert": "error", "oxc/approx-constant": "warn", "no-plusplus": "off", @@ -54,6 +56,45 @@ "maple/no-ordie-compiled-query": "off" } }, + // A test asserts on a fixture it just built, so `!` there documents the fixture + // rather than hiding an unchecked value; and a test that drives a throwing + // boundary on purpose needs `try`/`catch` to observe it. + { + "files": [ + "**/*.test.ts", + "**/*.test.tsx", + "**/*.spec.ts", + "**/*.spec.tsx", + "**/test/**", + "**/tests/**", + "**/__tests__/**" + ], + "rules": { + "maple/no-try-catch": "off", + "typescript/no-non-null-assertion": "off" + } + }, + // The rule points at an Effect primitive, so it only applies where Effect is on + // the dependency list. These three ship to customers with no runtime deps at all + // (`@maple/browser` and `@maple/browser-session` are under a bundle-size budget), + // so `try`/`catch` is the only error handling they have. + { + "files": ["packages/browser/**", "packages/browser-session/**", "packages/clickhouse-cli/**"], + "rules": { + "maple/no-try-catch": "off" + } + }, + // Burndown list, not a policy. `packages/*` and `lib/*` source is clean and + // gated for both rules; the apps and the one-shot ops scripts still hold + // ~460 `try`/`catch` blocks and ~500 non-null assertions between them. + // Drop entries from this list as each is converted rather than widening it. + { + "files": ["apps/**", "scripts/**", "**/scripts/**", "examples/**", "alchemy.run.ts"], + "rules": { + "maple/no-try-catch": "off", + "typescript/no-non-null-assertion": "off" + } + }, // Standalone generic DSLs use `any` as a variance placeholder; Maple product code may not. { "files": ["lib/**"], diff --git a/lib/cache/src/edge-cache.ts b/lib/cache/src/edge-cache.ts index a8cb66a81..1dc5ced4c 100644 --- a/lib/cache/src/edge-cache.ts +++ b/lib/cache/src/edge-cache.ts @@ -114,8 +114,8 @@ const sha256Hex = async (input: string): Promise => { const digest = await crypto.subtle.digest("SHA-256", bytes) const view = new Uint8Array(digest) let out = "" - for (let i = 0; i < view.length; i++) { - out += view[i]!.toString(16).padStart(2, "0") + for (const byte of view) { + out += byte.toString(16).padStart(2, "0") } return out } diff --git a/lib/clickhouse-builder/src/ch/compile.ts b/lib/clickhouse-builder/src/ch/compile.ts index c11180d1d..8eafd68e6 100644 --- a/lib/clickhouse-builder/src/ch/compile.ts +++ b/lib/clickhouse-builder/src/ch/compile.ts @@ -250,8 +250,9 @@ export interface RowSchemaMismatch { const structFieldNames = (schema: unknown): ReadonlyArray | undefined => { const ast = (schema as { readonly ast?: { readonly _tag?: string } } | undefined)?.ast if (ast?._tag !== "Objects") return undefined - const signatures = (ast as { readonly propertySignatures?: ReadonlyArray<{ readonly name: PropertyKey }> }) - .propertySignatures + const signatures = ( + ast as { readonly propertySignatures?: ReadonlyArray<{ readonly name: PropertyKey }> } + ).propertySignatures return signatures?.map((signature) => String(signature.name)) } @@ -865,7 +866,8 @@ const deriveUnionRowSchema = ( const fields: Record> = {} for (const [alias, schemas] of perColumn) { - fields[alias] = schemas.length === 1 ? schemas[0]! : Schema.Union(schemas) + const only = schemas.length === 1 ? schemas[0] : undefined + fields[alias] = only ?? Schema.Union(schemas) } return { schema: Schema.Struct(fields) } } diff --git a/lib/clickhouse-builder/src/ch/define-fn.ts b/lib/clickhouse-builder/src/ch/define-fn.ts index 2f96aaf17..098d5015c 100644 --- a/lib/clickhouse-builder/src/ch/define-fn.ts +++ b/lib/clickhouse-builder/src/ch/define-fn.ts @@ -55,7 +55,8 @@ export const withoutNull = ( const rest = ast.types.filter((type) => type._tag !== "Null") if (rest.length === ast.types.length || rest.length === 0) return undefined const members = rest.map((type) => Schema.make(type)) - return (members.length === 1 ? members[0]! : Schema.Union(members)) as Schema.Codec + const only = members.length === 1 ? members[0] : undefined + return (only ?? Schema.Union(members)) as Schema.Codec } // Re-export for consumer convenience diff --git a/lib/effect-cloudflare/src/fetcher.ts b/lib/effect-cloudflare/src/fetcher.ts index 24ff77cc6..c15022bd4 100644 --- a/lib/effect-cloudflare/src/fetcher.ts +++ b/lib/effect-cloudflare/src/fetcher.ts @@ -208,14 +208,16 @@ export const fromCloudflareSocket = (cfSocket: cf.Socket): Socket.Socket => { latch.whenOpen( Effect.suspend(() => { if (Socket.isCloseEvent(chunk)) { - return Deferred.fail(currentFiberSet!.deferred, closeError(chunk.code, chunk.reason)) - } - if (!writerRef) { - writerRef = cfSocket.writable.getWriter() + // `latch.whenOpen` only admits a write while a run is in flight, and + // a run is exactly what sets `currentFiberSet`. + const fiberSet = currentFiberSet + if (fiberSet === undefined) return Effect.void + return Deferred.fail(fiberSet.deferred, closeError(chunk.code, chunk.reason)) } + const writer = (writerRef ??= cfSocket.writable.getWriter()) const data = typeof chunk === "string" ? encoder.encode(chunk) : chunk return Effect.tryPromise({ - try: () => writerRef!.write(data), + try: () => writer.write(data), catch: (cause) => new Socket.SocketError({ reason: new Socket.SocketWriteError({ cause }), diff --git a/lib/effect-cloudflare/src/runtime.ts b/lib/effect-cloudflare/src/runtime.ts index 00d67952a..41ac98085 100644 --- a/lib/effect-cloudflare/src/runtime.ts +++ b/lib/effect-cloudflare/src/runtime.ts @@ -1,5 +1,5 @@ -import type { Context, Effect } from "effect" -import { Cause, ConfigProvider, Exit, Layer, ManagedRuntime } from "effect" +import type { Context } from "effect" +import { Cause, ConfigProvider, Effect, Exit, Layer, ManagedRuntime, Result } from "effect" /** * Minimal shape of CF `ExecutionContext.waitUntil`. Accept any structurally @@ -49,10 +49,13 @@ export const buildRequestRuntime = ( }) const flush = async () => { await drainScheduler() - try { - await runtime.dispose() - } catch (err) { - console.error("[effect-cloudflare] runtime flush failed:", err) + // `flush` runs inside `ctx.waitUntil`, where a rejection surfaces as a + // Worker error caused purely by teardown. + const disposed = await Effect.runPromise( + Effect.result(Effect.tryPromise({ try: () => runtime.dispose(), catch: (cause) => cause })), + ) + if (Result.isFailure(disposed)) { + console.error("[effect-cloudflare] runtime flush failed:", disposed.failure) } } return { services, flush } @@ -80,12 +83,12 @@ export const withRequestRuntime = , Ctx e const response = handler(request, resolvedServices, env, ctx) ctx.waitUntil( (async () => { - try { - await response - } catch { - // Swallow handler errors — the handler's own error path is - // responsible for surfacing them. - } + // Wait for the response without adopting its rejection — the + // handler's own error path is responsible for surfacing that; here + // it only marks the point where the scope may close. + await Effect.runPromise( + Effect.ignore(Effect.tryPromise({ try: () => response, catch: (cause) => cause })), + ) await flush() })(), ) diff --git a/lib/effect-db/src/atom/AtomTanStackDB.ts b/lib/effect-db/src/atom/AtomTanStackDB.ts index 9312ef200..4e19983bc 100644 --- a/lib/effect-db/src/atom/AtomTanStackDB.ts +++ b/lib/effect-db/src/atom/AtomTanStackDB.ts @@ -122,7 +122,7 @@ export const makeSingleCollectionAtom = 0 ? entries[0]![1] : undefined + const newData = entries[0]?.[1] get.setSelf(AsyncResult.success(newData)) }) @@ -149,7 +149,7 @@ export const makeSingleCollectionAtom = 0 ? entries[0]![1] : undefined + const initialData = entries[0]?.[1] return AsyncResult.success(initialData) }) diff --git a/lib/effect-db/src/electric/optimistic-action.ts b/lib/effect-db/src/electric/optimistic-action.ts index ccad48591..0148a7c9f 100644 --- a/lib/effect-db/src/electric/optimistic-action.ts +++ b/lib/effect-db/src/electric/optimistic-action.ts @@ -253,17 +253,23 @@ export function optimisticAction< mutateResult = onMutate(variables) }) - yield* Effect.tryPromise({ - try: () => transaction.isPersisted.promise, - catch: (error) => { + // Roll back before mapping, as its own step in the chain: the rollback is + // best effort, and a rollback that throws must not displace the mutation + // failure the caller is owed. + const rollbackIfPending = Effect.ignore( + Effect.try(() => { if (transaction.state !== "completed" && transaction.state !== "failed") { - try { - transaction.rollback() - } catch { - // Best effort; preserve the mutation failure below. - } + transaction.rollback() } + }), + ) + yield* Effect.tryPromise({ + try: () => transaction.isPersisted.promise, + catch: (error) => error, + }).pipe( + Effect.tapError(() => rollbackIfPending), + Effect.mapError((error) => { if (error && typeof error === "object" && "_tag" in error) { return error as TError | SyncError } @@ -272,8 +278,8 @@ export function optimisticAction< message: error instanceof Error ? error.message : "Optimistic action failed", cause: error, }) - }, - }) + }), + ) return { data: mutationResult.data, diff --git a/packages/auth/src/index.ts b/packages/auth/src/index.ts index 4fca3a689..4d4ecac33 100644 --- a/packages/auth/src/index.ts +++ b/packages/auth/src/index.ts @@ -687,7 +687,8 @@ export const makeResolveTenant = ( ? verifyOrgMembership : undefined - if (!auth.orgId && !orgIdOverride) { + const sessionOrgId = orgIdOverride ?? auth.orgId + if (!sessionOrgId) { // No active organization in the session. A request that names one it // can prove membership of is still serviceable — this is the widget // publishing path, whose whole point is not to disturb whatever the @@ -705,10 +706,7 @@ export const makeResolveTenant = ( } const clerkTenant: TenantContext = { - orgId: yield* decodeOrgId( - orgIdOverride ?? auth.orgId!, - "Invalid organization in Clerk session token", - ), + orgId: yield* decodeOrgId(sessionOrgId, "Invalid organization in Clerk session token"), userId, roles: typeof auth.orgRole === "string" diff --git a/packages/clickhouse-cli/src/cli.ts b/packages/clickhouse-cli/src/cli.ts index 8dc2dee86..83b126e6f 100644 --- a/packages/clickhouse-cli/src/cli.ts +++ b/packages/clickhouse-cli/src/cli.ts @@ -223,7 +223,7 @@ interface Flags { function parseFlags(args: ReadonlyArray): Flags { const flags: Record = {} for (let i = 0; i < args.length; i++) { - const a = args[i]! + const a = args[i] ?? "" if (!a.startsWith("--")) { continue } diff --git a/packages/domain/src/chat-session.ts b/packages/domain/src/chat-session.ts index c373dce42..be14d82cf 100644 --- a/packages/domain/src/chat-session.ts +++ b/packages/domain/src/chat-session.ts @@ -14,7 +14,7 @@ * *model* stream, while this is a *session* stream that also carries user turns, approval gates and * turn lifecycle. `apps/api/src/chat/events.ts` is the only place the two are mapped. */ -import { Schema } from "effect" +import { Option, Schema } from "effect" import { ActorId, AuthMode, OrgId, RoleName, UserId } from "./primitives" // Session addressing @@ -337,13 +337,10 @@ export const decodeChatEventOrThrow = Schema.decodeUnknownSync(Schema.fromJsonSt * unrecognised frame instead means adding a new `ChatEvent` member degrades old clients rather than * bricking them. */ -export const decodeChatEvent = (frame: string): ChatEvent | undefined => { - try { - return decodeChatEventOrThrow(frame) - } catch { - return undefined - } -} +const decodeChatEventOption = Schema.decodeUnknownOption(Schema.fromJsonString(ChatEvent)) + +export const decodeChatEvent = (frame: string): ChatEvent | undefined => + Option.getOrUndefined(decodeChatEventOption(frame)) /** * Durable-storage codec for the event log. diff --git a/packages/domain/src/clickhouse/apply-plan.ts b/packages/domain/src/clickhouse/apply-plan.ts index 67fd3ff0a..c1455fe14 100644 --- a/packages/domain/src/clickhouse/apply-plan.ts +++ b/packages/domain/src/clickhouse/apply-plan.ts @@ -8,6 +8,8 @@ * step's `sql` in order; the Workflow additionally wraps each in a durable * `step.do(step.name, …)` for resumability + progress. */ +import { Option, Schema } from "effect" + import { compileBackfillChunk, isBackfill, @@ -55,15 +57,18 @@ const toChDateTime = (unixSeconds: number): string => { return `${d.getUTCFullYear()}-${p(d.getUTCMonth() + 1)}-${p(d.getUTCDate())} ${p(d.getUTCHours())}:${p(d.getUTCMinutes())}:${p(d.getUTCSeconds())}` } +const decodeJsonRow = Schema.decodeUnknownOption( + Schema.fromJsonString(Schema.Record(Schema.String, Schema.Unknown)), +) + const parseFirstRow = (text: string): Record | null => { for (const line of text.split("\n")) { const trimmed = line.trim() if (trimmed.length === 0) continue - try { - return JSON.parse(trimmed) as Record - } catch { - // ignore — controlled query - } + // The query is ours and asks for JSONEachRow, so a line that does not decode + // is a warning or a blank the server interleaved — skip to the next one. + const row = decodeJsonRow(trimmed) + if (Option.isSome(row)) return row.value } return null } diff --git a/packages/domain/src/clickhouse/features.ts b/packages/domain/src/clickhouse/features.ts index d30b8b815..ab8b2abbc 100644 --- a/packages/domain/src/clickhouse/features.ts +++ b/packages/domain/src/clickhouse/features.ts @@ -59,11 +59,15 @@ const parseVersion = (version: string): readonly [number, number, number] => { } export const clickHouseVersionAtLeast = (actual: string, minimum: string): boolean => { - const left = parseVersion(actual) - const right = parseVersion(minimum) - for (let i = 0; i < 3; i++) { - if (left[i]! > right[i]!) return true - if (left[i]! < right[i]!) return false + const [leftMajor, leftMinor, leftPatch] = parseVersion(actual) + const [rightMajor, rightMinor, rightPatch] = parseVersion(minimum) + for (const [left, right] of [ + [leftMajor, rightMajor], + [leftMinor, rightMinor], + [leftPatch, rightPatch], + ] as const) { + if (left > right) return true + if (left < right) return false } return true } diff --git a/packages/domain/src/http/v2/auth.ts b/packages/domain/src/http/v2/auth.ts index 354d45631..37e0d34ab 100644 --- a/packages/domain/src/http/v2/auth.ts +++ b/packages/domain/src/http/v2/auth.ts @@ -120,13 +120,13 @@ const isReadOnlyPost = (path: string): boolean => * read access; mutation methods require write access. Returns null for non-/v2 paths. */ export const requiredScopeForRequest = (method: string, path: string): RequiredScope | null => { - const match = /^\/v2\/([a-z][a-z0-9_]*)(?:\/|$)/.exec(path) - if (match === null) return null + const [, family] = /^\/v2\/([a-z][a-z0-9_]*)(?:\/|$)/.exec(path) ?? [] + if (family === undefined) return null const access = method === "GET" || method === "HEAD" || (method === "POST" && isReadOnlyPost(path)) ? "read" : "write" - return { family: match[1]!, access } + return { family, access } } /** diff --git a/packages/domain/src/http/v2/envelopes.ts b/packages/domain/src/http/v2/envelopes.ts index 201a5c097..acc25d35b 100644 --- a/packages/domain/src/http/v2/envelopes.ts +++ b/packages/domain/src/http/v2/envelopes.ts @@ -115,9 +115,9 @@ export const ListOf = (item: S) => export const encodeOffsetCursor = (offset: number): string => `off_${offset.toString(36)}` export const decodeOffsetCursor = (cursor: string): number | null => { - const match = /^off_([0-9a-z]+)$/.exec(cursor) - if (match === null) return null - const offset = Number.parseInt(match[1]!, 36) + const [, digits] = /^off_([0-9a-z]+)$/.exec(cursor) ?? [] + if (digits === undefined) return null + const offset = Number.parseInt(digits, 36) return Number.isSafeInteger(offset) && offset >= 0 ? offset : null } diff --git a/packages/domain/src/http/v2/public-id.ts b/packages/domain/src/http/v2/public-id.ts index 6136f9d06..7d232900c 100644 --- a/packages/domain/src/http/v2/public-id.ts +++ b/packages/domain/src/http/v2/public-id.ts @@ -1,4 +1,4 @@ -import { Effect, Schema, SchemaAST, SchemaGetter, SchemaIssue } from "effect" +import { Effect, Option, Schema, SchemaAST, SchemaGetter, SchemaIssue } from "effect" /** * Stripe-style prefixed public object IDs for the v2 API. @@ -57,9 +57,9 @@ const base58Encode = (bytes: Uint8Array): string => { const digits: number[] = [] for (let i = zeros; i < bytes.length; i++) { - let carry = bytes[i]! + let carry = bytes[i] ?? 0 for (let j = 0; j < digits.length; j++) { - carry += digits[j]! << 8 + carry += (digits[j] ?? 0) << 8 digits[j] = carry % 58 carry = (carry / 58) | 0 } @@ -70,7 +70,9 @@ const base58Encode = (bytes: Uint8Array): string => { } let out = "1".repeat(zeros) - for (let i = digits.length - 1; i >= 0; i--) out += ALPHABET[digits[i]!] + // `charAt` rather than `[]`: every digit is already `% 58`, and it keeps the + // expression a `string` instead of a `string | undefined` to unwrap. + for (let i = digits.length - 1; i >= 0; i--) out += ALPHABET.charAt(digits[i] ?? 0) return out } @@ -82,11 +84,11 @@ const base58Decode = (input: string): Uint8Array | null => { const bytes: number[] = [] for (let i = zeros; i < input.length; i++) { - const value = ALPHABET_MAP.get(input[i]!) + const value = ALPHABET_MAP.get(input.charAt(i)) if (value === undefined) return null let carry = value for (let j = 0; j < bytes.length; j++) { - carry += bytes[j]! * 58 + carry += (bytes[j] ?? 0) * 58 bytes[j] = carry & 0xff carry >>= 8 } @@ -97,7 +99,7 @@ const base58Decode = (input: string): Uint8Array | null => { } const out = new Uint8Array(zeros + bytes.length) - for (let i = 0; i < bytes.length; i++) out[zeros + i] = bytes[bytes.length - 1 - i]! + for (let i = 0; i < bytes.length; i++) out[zeros + i] = bytes[bytes.length - 1 - i] ?? 0 return out } @@ -138,18 +140,21 @@ export const decodePublicId = (prefix: PublicIdPrefix, publicId: string): string const bytes = base58Decode(body) if (bytes === null || bytes.length < 2) return null - const mode = bytes[0]! + const mode = bytes[0] const idBytes = bytes.subarray(1) if (mode === MODE_UUID) { if (idBytes.length !== 16) return null return bytesToUuid(idBytes) } if (mode === MODE_UTF8) { - try { - return new TextDecoder("utf-8", { fatal: true }).decode(idBytes) - } catch { - return null - } + // `fatal` makes the decoder throw on an invalid sequence rather than + // silently emitting U+FFFD, which would turn a corrupt ID into a + // plausible-looking one. + return Option.getOrNull( + Effect.runSync( + Effect.option(Effect.try(() => new TextDecoder("utf-8", { fatal: true }).decode(idBytes))), + ), + ) } return null } diff --git a/packages/domain/src/tinybird/fingerprint.ts b/packages/domain/src/tinybird/fingerprint.ts index f6af87776..027bedc63 100644 --- a/packages/domain/src/tinybird/fingerprint.ts +++ b/packages/domain/src/tinybird/fingerprint.ts @@ -11,6 +11,8 @@ * grouping quality. */ +import { Option, Schema } from "effect" + export interface FingerprintInputs { /** First normalized frame — stored on error_events and error_issues for display. */ readonly topFrame: string @@ -207,15 +209,14 @@ const LABEL_KEYS = ["title", "message", "error", "_tag", "reason", "name"] as co * Parity caveat: JS `JSON.parse` is stricter than ClickHouse `isValidJSON`; * acceptable for the well-formed RFC7807 / serialized-error messages we see. */ +const decodeJsonObject = Schema.decodeUnknownOption( + Schema.fromJsonString(Schema.Record(Schema.String, Schema.Unknown)), +) + function tryParseJsonObject(s: string): Record | undefined { - try { - const v = JSON.parse(s) as unknown - return v !== null && typeof v === "object" && !Array.isArray(v) - ? (v as Record) - : undefined - } catch { - return undefined - } + // `Schema.Record` rejects arrays and scalars, so the non-object cases the SQL + // gate excludes decode to `None` alongside the malformed ones. + return Option.getOrUndefined(decodeJsonObject(s)) } /** diff --git a/packages/effect-sdk/src/client/consent-http-client.ts b/packages/effect-sdk/src/client/consent-http-client.ts index 6aaf50b0f..9abb0bc93 100644 --- a/packages/effect-sdk/src/client/consent-http-client.ts +++ b/packages/effect-sdk/src/client/consent-http-client.ts @@ -1,5 +1,7 @@ import { consentAllowedSince, hasConsent } from "@maple/browser-session" import { Effect, Layer } from "effect" + +import { trySyncOrUndefined } from "../shared/try-sync.js" import { FetchHttpClient, HttpBody, @@ -35,13 +37,11 @@ interface OtlpBody { readonly resourceLogs?: ReadonlyArray } -const after = (value: unknown, threshold: bigint): boolean => { - try { - return BigInt(String(value ?? 0)) >= threshold - } catch { - return false - } -} +// A nanosecond timestamp that is not a numeric string throws out of `BigInt` +// rather than producing NaN, and an event with no readable timestamp cannot be +// shown to postdate consent — so it is dropped. +const after = (value: unknown, threshold: bigint): boolean => + trySyncOrUndefined(() => BigInt(String(value ?? 0)) >= threshold) ?? false const pruneTraces = (body: OtlpBody, threshold: bigint): OtlpBody | undefined => { const resourceSpans = (body.resourceSpans ?? []) @@ -87,12 +87,17 @@ export const filterOtlpRequestForConsent = ( if (!requireConsent) return request if (request.url.endsWith("/v1/metrics")) return undefined if (request.body._tag !== "Uint8Array") return undefined + // Bound before the closure: narrowing on `request.body` does not survive into one. + const body = request.body const since = consentAllowedSince() if (!Number.isFinite(since)) return undefined - try { - // SAFETY: The SDK's OTLP encoder created this body; malformed shapes throw below and fail closed. - const decoded = JSON.parse(new TextDecoder().decode(request.body.body)) as OtlpBody + // Unknown payloads must fail closed on an explicit-consent install: an + // undecodable body, or a `since` past the BigInt range, drops the request + // rather than forwarding it unfiltered. + return trySyncOrUndefined(() => { + // SAFETY: The SDK's OTLP encoder created this body; malformed shapes throw here and fail closed. + const decoded = JSON.parse(new TextDecoder().decode(body.body)) as OtlpBody const threshold = BigInt(Math.trunc(since)) * 1_000_000n const filtered = request.url.endsWith("/v1/traces") ? pruneTraces(decoded, threshold) @@ -100,12 +105,9 @@ export const filterOtlpRequestForConsent = ( ? pruneLogs(decoded, threshold) : undefined return filtered - ? HttpClientRequest.setBody(request, HttpBody.jsonUnsafe(filtered, request.body.contentType)) + ? HttpClientRequest.setBody(request, HttpBody.jsonUnsafe(filtered, body.contentType)) : undefined - } catch { - // Unknown payloads must fail closed on an explicit-consent install. - return undefined - } + }) } export const consentHttpClientLayer = (requireConsent: boolean) => diff --git a/packages/effect-sdk/src/client/flushable.ts b/packages/effect-sdk/src/client/flushable.ts index 37faf36d5..8a769db17 100644 --- a/packages/effect-sdk/src/client/flushable.ts +++ b/packages/effect-sdk/src/client/flushable.ts @@ -9,6 +9,7 @@ import { Layer, Redacted } from "effect" import { buildResolved, type FlushTransport, + guardFlush, makeSerializedFlush, type Resolved, type ResourceInput, @@ -19,6 +20,7 @@ import { type LogBuffer, makeLogBuffer } from "../shared/flushable-logger.js" import { makeMetricBuffer } from "../shared/flushable-metrics.js" import { type CaptureExceptionOptions, makeSpanBuffer, type SpanBuffer } from "../shared/flushable-tracer.js" import { browserDocument, browserNavigator } from "./browser-globals.js" +import { trySyncOrUndefined } from "../shared/try-sync.js" import { type ClientReplayConfig, startClientSession } from "./replay-loader.js" import { withSessionLink } from "./session-link.js" import type { PrivacyOptions } from "./track.js" @@ -160,9 +162,10 @@ const buildBrowserAttributes = (config: MapleClientFlushableConfig): Record Intl.DateTimeFormat().resolvedOptions().timeZone) + if (timezone) attributes["browser.timezone"] = timezone } if (config.environment) { // Dual-emit: legacy key (pre-extracted by Tinybird MVs) + the canonical @@ -243,8 +246,8 @@ export const make = (config: MapleClientFlushableConfig): FlushableTelemetry => // Never rejects — fired from `pagehide`/`visibilitychange` handlers and the // auto-flush timer as `void flush()`. - const flush = makeSerializedFlush(async (): Promise => { - try { + const flush = makeSerializedFlush( + guardFlush("[MapleClientSDK]", async (): Promise => { if (!hasConsent()) { spans.drain() logs.drain() @@ -263,10 +266,8 @@ export const make = (config: MapleClientFlushableConfig): FlushableTelemetry => logPrefix: "[MapleClientSDK]", onNoOp: noOpNotice, }) - } catch (err) { - console.error("[MapleClientSDK] flush failed:", err) - } - }) + }), + ) const intervalMs = config.autoFlushInterval === undefined diff --git a/packages/effect-sdk/src/client/layer.ts b/packages/effect-sdk/src/client/layer.ts index 5911c634a..f48df1722 100644 --- a/packages/effect-sdk/src/client/layer.ts +++ b/packages/effect-sdk/src/client/layer.ts @@ -1,6 +1,7 @@ import type { Duration } from "effect" import { Effect, Layer } from "effect" import { Otlp } from "effect/unstable/observability" +import { trySyncOrUndefined } from "../shared/try-sync.js" import { browserNavigator } from "./browser-globals.js" import { consentHttpClientLayer } from "./consent-http-client.js" import { type ClientReplayConfig, startClientSession } from "./replay-loader.js" @@ -90,9 +91,10 @@ export const layer = (config: MapleClientConfig) => { if (nav.language) attributes["browser.language"] = nav.language } if (typeof Intl !== "undefined") { - try { - attributes["browser.timezone"] = Intl.DateTimeFormat().resolvedOptions().timeZone - } catch {} + // A locale-stripped build throws from `DateTimeFormat` rather than + // reporting an unknown zone. + const timezone = trySyncOrUndefined(() => Intl.DateTimeFormat().resolvedOptions().timeZone) + if (timezone) attributes["browser.timezone"] = timezone } if (config.environment) { // Dual-emit: legacy key (pre-extracted by Tinybird MVs) + the canonical diff --git a/packages/effect-sdk/src/client/replay-loader.ts b/packages/effect-sdk/src/client/replay-loader.ts index 8b34cee74..fa48cd698 100644 --- a/packages/effect-sdk/src/client/replay-loader.ts +++ b/packages/effect-sdk/src/client/replay-loader.ts @@ -70,11 +70,12 @@ const noOpHandle: ClientSessionHandle = { stop: () => Promise.resolve() } export const startClientSession = (config: ClientSessionConfig): ClientSessionHandle => { configurePrivacy(config.privacy) if (!hasConsent()) clearPendingEvents() - if (typeof window === "undefined" || !config.ingestKey || readSessionSink()) return noOpHandle + const ingestKey = config.ingestKey + if (typeof window === "undefined" || !ingestKey || readSessionSink()) return noOpHandle const engineConfig = { endpoint: config.endpoint.replace(/\/$/, ""), - ingestKey: config.ingestKey, + ingestKey, sdk: CLIENT_SDK_HINT, maskAllInputs: config.replay?.maskAllInputs ?? true, maskAllText: config.replay?.maskAllText ?? false, @@ -124,7 +125,7 @@ export const startClientSession = (config: ClientSessionConfig): ClientSessionHa } next.replay = startReplaySession({ endpoint: config.endpoint, - ingestKey: config.ingestKey!, + ingestKey, sdk: CLIENT_SDK_HINT, serviceName: config.serviceName, environment: config.environment, diff --git a/packages/effect-sdk/src/cloudflare/index.ts b/packages/effect-sdk/src/cloudflare/index.ts index b7f142be2..608d23d85 100644 --- a/packages/effect-sdk/src/cloudflare/index.ts +++ b/packages/effect-sdk/src/cloudflare/index.ts @@ -33,6 +33,7 @@ import { Layer } from "effect" import { buildResolved, fetchTransport, + guardFlush, makeSerializedFlush, type Resolved, runFlush, @@ -163,8 +164,8 @@ export const make = (config: Config = {}): Telemetry => { // Never rejects: this runs inside `ctx.waitUntil`, where a rejection would // surface as an unhandled Worker error caused purely by telemetry. - const flush = makeSerializedFlush(async (env: Record): Promise => { - try { + const flush = makeSerializedFlush( + guardFlush("[MapleCloudflareSDK]", async (env: Record): Promise => { // Effect defers work onto the scheduler's next macrotask // (`scheduleTask(task, 0)`) — including `HttpMiddleware.tracer`'s // `span.end` and `withSpan` finalizers — while the drain below is @@ -192,10 +193,8 @@ export const make = (config: Config = {}): Telemetry => { logPrefix: "[MapleCloudflareSDK]", onNoOp: noOpNotice, }) - } catch (err) { - console.error("[MapleCloudflareSDK] flush failed:", err) - } - }) + }), + ) return { layer, flush } } diff --git a/packages/effect-sdk/src/server/config.ts b/packages/effect-sdk/src/server/config.ts index bfafd92b8..50df32e7a 100644 --- a/packages/effect-sdk/src/server/config.ts +++ b/packages/effect-sdk/src/server/config.ts @@ -1,5 +1,7 @@ import { Config, Effect, Option } from "effect" +import { trySyncOrUndefined } from "../shared/try-sync.js" + /** * Resolve the ingest endpoint. * @@ -106,14 +108,9 @@ export const parseOtelResourceAttributes = (input: string): Record decodeURIComponent(raw)) ?? raw } return result } diff --git a/packages/effect-sdk/src/server/container.ts b/packages/effect-sdk/src/server/container.ts index fe430eb8f..d4f1eb301 100644 --- a/packages/effect-sdk/src/server/container.ts +++ b/packages/effect-sdk/src/server/container.ts @@ -19,6 +19,8 @@ // build. `process.getBuiltinModule` (Node ≥ 20.16, Bun) loads them lazily and // synchronously; when it's absent, detection just reports nothing. +import { trySyncOrUndefined } from "../shared/try-sync.js" + const CONTAINER_ID_RE = /\/docker\/containers\/([0-9a-f]{64})\// const CGROUP_ID_RE = /([0-9a-f]{64})/ const SHORT_ID_HOSTNAME_RE = /^[0-9a-f]{12}$/ @@ -35,7 +37,7 @@ export interface ContainerProbe { * guarded by the caller (`getContainerAttributes` wraps the whole thing). */ export const deriveContainerAttributes = (probe: ContainerProbe): Record => { - const inDocker = safe(() => probe.exists("/.dockerenv")) ?? false + const inDocker = trySyncOrUndefined(() => probe.exists("/.dockerenv")) ?? false const attrs: Record = {} if (inDocker) attrs["container.runtime"] = "docker" @@ -44,10 +46,10 @@ export const deriveContainerAttributes = (probe: ContainerProbe): Record probe.readFile("/proc/self/mountinfo").match(CONTAINER_ID_RE)?.[1]) ?? - safe(() => probe.readFile("/proc/self/cgroup").match(CGROUP_ID_RE)?.[1]) ?? + trySyncOrUndefined(() => probe.readFile("/proc/self/mountinfo").match(CONTAINER_ID_RE)?.[1]) ?? + trySyncOrUndefined(() => probe.readFile("/proc/self/cgroup").match(CGROUP_ID_RE)?.[1]) ?? (inDocker - ? safe(() => { + ? trySyncOrUndefined(() => { const name = probe.hostname() return SHORT_ID_HOSTNAME_RE.test(name) ? name : undefined }) @@ -56,14 +58,6 @@ export const deriveContainerAttributes = (probe: ContainerProbe): Record(fn: () => A): A | undefined => { - try { - return fn() - } catch { - return undefined - } -} - type FsModule = { existsSync: (path: string) => boolean; readFileSync: (path: string, enc: string) => string } type OsModule = { hostname: () => string } @@ -91,8 +85,8 @@ export const getContainerAttributes = (): Record => { if (proc?.platform !== "linux" || typeof loadBuiltin !== "function") { return (cached = {}) } - const fs = safe(() => loadBuiltin("node:fs")) - const os = safe(() => loadBuiltin("node:os")) + const fs = trySyncOrUndefined(() => loadBuiltin("node:fs")) + const os = trySyncOrUndefined(() => loadBuiltin("node:os")) if (!fs || !os) return (cached = {}) return (cached = deriveContainerAttributes({ diff --git a/packages/effect-sdk/src/server/flushable.ts b/packages/effect-sdk/src/server/flushable.ts index 39ff16f49..fb512d512 100644 --- a/packages/effect-sdk/src/server/flushable.ts +++ b/packages/effect-sdk/src/server/flushable.ts @@ -19,6 +19,7 @@ import { Effect, Layer } from "effect" import { buildResolved, fetchTransport, + guardFlush, makeSerializedFlush, type Resolved, runFlush, @@ -153,14 +154,12 @@ export const make = (config: MapleFlushableConfig = {}): FlushableTelemetry => { return resolvedPromise } - // `flush` is documented to never reject: callers `await` it at shutdown and - // the auto-flush timer fires it as `void flush()`, where a rejection would be - // an unhandled rejection every tick (fatal under - // `--unhandled-rejections=strict`). `runFlush` already swallows per-signal - // transport errors; this catch covers resource resolution, which runs before - // it. - const flush = makeSerializedFlush(async (): Promise => { - try { + // `guardFlush` is what makes `flush` documented-never-rejects hold: callers + // `await` it at shutdown and the auto-flush timer fires it as `void flush()`. + // It covers resource resolution, which runs before `runFlush` absorbs the + // per-signal transport errors. + const flush = makeSerializedFlush( + guardFlush("[MapleServerSDK]", async (): Promise => { const resolved = await ensureResolved() await runFlush({ resolved, @@ -174,10 +173,8 @@ export const make = (config: MapleFlushableConfig = {}): FlushableTelemetry => { logPrefix: "[MapleServerSDK]", onNoOp: noOpNotice, }) - } catch (err) { - console.error("[MapleServerSDK] flush failed:", err) - } - }) + }), + ) const intervalMs = config.autoFlushInterval === undefined diff --git a/packages/effect-sdk/src/server/platform.ts b/packages/effect-sdk/src/server/platform.ts index ea73895c5..6362c0947 100644 --- a/packages/effect-sdk/src/server/platform.ts +++ b/packages/effect-sdk/src/server/platform.ts @@ -73,10 +73,10 @@ const archAttrs = (a: string): Attrs => ), } -const lambdaAttrs = (env: PlatformInputs["env"]): Attrs => ({ +const lambdaAttrs = (env: PlatformInputs["env"], functionName: string): Attrs => ({ "cloud.provider": "aws", "cloud.platform": "aws_lambda", - "faas.name": env.AWS_LAMBDA_FUNCTION_NAME!, + "faas.name": functionName, ...(env.AWS_LAMBDA_FUNCTION_VERSION && { "faas.version": env.AWS_LAMBDA_FUNCTION_VERSION }), ...(env.AWS_LAMBDA_LOG_STREAM_NAME && { "faas.instance": env.AWS_LAMBDA_LOG_STREAM_NAME }), ...(env.AWS_REGION @@ -205,7 +205,10 @@ export const derivePlatformAttributes = (inputs: PlatformInputs): PlatformAttrib // Lambda overrides std-env's provider when present; matches the original // short-circuit semantics. Otherwise std-env's provider drives cloud.*. - const cloudAttrs = env.AWS_LAMBDA_FUNCTION_NAME ? lambdaAttrs(env) : providerAttrs(prov, env) + // The presence of the function name IS the Lambda detection, so hand it to + // `lambdaAttrs` rather than have it look the same variable up again. + const lambdaFunctionName = env.AWS_LAMBDA_FUNCTION_NAME + const cloudAttrs = lambdaFunctionName ? lambdaAttrs(env, lambdaFunctionName) : providerAttrs(prov, env) const cloudResolved = "cloud.provider" in cloudAttrs return { diff --git a/packages/effect-sdk/src/shared/flush-core.ts b/packages/effect-sdk/src/shared/flush-core.ts index 8489ede8a..4fa5de826 100644 --- a/packages/effect-sdk/src/shared/flush-core.ts +++ b/packages/effect-sdk/src/shared/flush-core.ts @@ -6,7 +6,7 @@ // resolution (env-lazy on Workers, env-auto-detect on Node, programmatic in the // browser) and its transport (plain `fetch` vs `fetch(keepalive)`); everything // downstream of a resolved endpoint lives here. -import { Redacted } from "effect" +import { Effect, Redacted, Result } from "effect" import type { LogBuffer, LogRecord } from "./flushable-logger.js" import type { MetricBuffer } from "./flushable-metrics.js" import type { OtlpSpan, SpanBuffer } from "./flushable-tracer.js" @@ -164,15 +164,40 @@ const flushSignal = async (args: { state.disabledUntil = 0 const batch = buffer.drain() if (batch.length === 0) return - try { - await transport.post(url, headers, body(batch)) - } catch (err) { + const posted = await Effect.runPromise( + Effect.result( + Effect.tryPromise({ + try: () => transport.post(url, headers, body(batch)), + catch: (cause) => cause, + }), + ), + ) + if (Result.isFailure(posted)) { buffer.restore(batch) state.disabledUntil = Date.now() + COOLDOWN_MS - console.error(`${logPrefix} ${signal} flush failed; cooldown 60s:`, err) + console.error(`${logPrefix} ${signal} flush failed; cooldown 60s:`, posted.failure) } } +/** + * Wrap a flush body so it logs its failure rather than rejecting. + * + * Every flush is documented never to reject: they are fired as `void flush()` + * from auto-flush timers, `pagehide`/`visibilitychange` handlers, and + * `ctx.waitUntil`, where a rejection is an unhandled rejection — fatal under + * `--unhandled-rejections=strict` — caused purely by telemetry. `runFlush` + * already absorbs per-signal transport errors; this covers everything around it, + * chiefly resource resolution. + */ +export const guardFlush = + >(logPrefix: string, run: (...args: Args) => Promise) => + async (...args: Args): Promise => { + const outcome = await Effect.runPromise( + Effect.result(Effect.tryPromise({ try: () => run(...args), catch: (cause) => cause })), + ) + if (Result.isFailure(outcome)) console.error(`${logPrefix} flush failed:`, outcome.failure) + } + /** Serialize flush calls so concurrent timers/manual hooks cannot drain overlapping batches. */ export const makeSerializedFlush = >( run: (...args: Args) => Promise, diff --git a/packages/effect-sdk/src/shared/try-sync.ts b/packages/effect-sdk/src/shared/try-sync.ts new file mode 100644 index 000000000..23f2caf7f --- /dev/null +++ b/packages/effect-sdk/src/shared/try-sync.ts @@ -0,0 +1,17 @@ +// A throwing host call, as a value. +// +// The SDK runs inside someone else's app, so it touches host APIs that throw +// rather than return a failure: `Intl` in a locale-stripped build, `BigInt` on a +// wire value that isn't numeric, `decodeURIComponent` on a literal `%`, a +// `node:fs` read the sandbox refuses. Every one of those is best-effort — the +// SDK degrades an attribute rather than breaking the host — and `Effect.try` is +// what makes that an `Option` the caller has to answer for instead of a `catch` +// block that quietly covers the next three statements too. +import { Effect, Option } from "effect" + +/** Run a throwing synchronous call, `None` if it threw. */ +export const trySync = (thunk: () => A): Option.Option => + Effect.runSync(Effect.option(Effect.try(thunk))) + +/** Run a throwing synchronous call, `undefined` if it threw. */ +export const trySyncOrUndefined = (thunk: () => A): A | undefined => Option.getOrUndefined(trySync(thunk)) diff --git a/packages/query-engine-integrations/src/ai/ai-integrations.ts b/packages/query-engine-integrations/src/ai/ai-integrations.ts index 48655ea0b..c49c1f68d 100644 --- a/packages/query-engine-integrations/src/ai/ai-integrations.ts +++ b/packages/query-engine-integrations/src/ai/ai-integrations.ts @@ -12,6 +12,8 @@ // `apps/ingest/src/ai_session.rs`). Attributes arrive as `Map(String, String)`, // so a missing key reads back as `''` and an undecodable value yields no field. +import { Effect, Option } from "effect" + import { AI_CORE_FIELDS, AI_GENAI_FIELDS, @@ -64,13 +66,8 @@ const readAttribute = (attributes: Record, key: string): string return value === undefined || value.trim() === "" ? undefined : value } -const parseJson = (raw: string): unknown => { - try { - return JSON.parse(raw) - } catch { - return undefined - } -} +const parseJson = (raw: string): unknown => + Option.getOrUndefined(Effect.runSync(Effect.option(Effect.try((): unknown => JSON.parse(raw))))) const decodeStringArray = (raw: string): readonly string[] | undefined => { // Real data carries both shapes for the same attribute: `'["stop"]'` from diff --git a/packages/query-engine-integrations/src/cloudflare/cloudflare-infra-filters.ts b/packages/query-engine-integrations/src/cloudflare/cloudflare-infra-filters.ts index ee7f0f426..1dc203be5 100644 --- a/packages/query-engine-integrations/src/cloudflare/cloudflare-infra-filters.ts +++ b/packages/query-engine-integrations/src/cloudflare/cloudflare-infra-filters.ts @@ -172,8 +172,9 @@ export const cloudflareFilterConditions = ( }) // `CH.when` only skips null/undefined, so an empty needle would compile to a match-everything // predicate — noise in the SQL and a needless change to the query fingerprint. - if (active("path") && (opts.pathContains ?? "") !== "") { - conditions.push(CH.positionCaseInsensitive(attrExpr($, "path"), CH.lit(opts.pathContains!)).gt(0)) + const pathContains = opts.pathContains ?? "" + if (active("path") && pathContains !== "") { + conditions.push(CH.positionCaseInsensitive(attrExpr($, "path"), CH.lit(pathContains)).gt(0)) } return conditions } diff --git a/packages/query-engine/src/caching/bucket-cache.ts b/packages/query-engine/src/caching/bucket-cache.ts index b4bf28e1c..4d311d1a6 100644 --- a/packages/query-engine/src/caching/bucket-cache.ts +++ b/packages/query-engine/src/caching/bucket-cache.ts @@ -84,8 +84,8 @@ const sha256Hex = async (input: string): Promise => { const digest = await crypto.subtle.digest("SHA-256", bytes) const view = new Uint8Array(digest) let out = "" - for (let i = 0; i < view.length; i++) { - out += view[i]!.toString(16).padStart(2, "0") + for (const byte of view) { + out += byte.toString(16).padStart(2, "0") } return out } @@ -569,9 +569,11 @@ export class BucketCacheService extends Context.Service ({ item, - points: freshByRange[index]!, + points: freshByRange[index] ?? [], })) const freshCachableBuckets = rangeResults.flatMap(({ item, points }) => item.cachable diff --git a/packages/query-engine/src/ch/pipe-dispatch.ts b/packages/query-engine/src/ch/pipe-dispatch.ts index 8bbb8a699..4986da07f 100644 --- a/packages/query-engine/src/ch/pipe-dispatch.ts +++ b/packages/query-engine/src/ch/pipe-dispatch.ts @@ -115,9 +115,28 @@ export function compilePipeQuery( const startTime = String(params.start_time ?? "2023-01-01 00:00:00") const endTime = String(params.end_time ?? "2099-12-31 23:59:59") const str = (key: string) => (params[key] != null ? String(params[key]) : undefined) - const int = (key: string, def?: number) => (params[key] != null ? Number(params[key]) : def) + // Overloaded rather than `def?: number`: with an optional default every + // defaulted call still typed as `number | undefined` and every call site paid + // for it with a `!`. + function int(key: string): number | undefined + function int(key: string, def: number): number + function int(key: string, def?: number): number | undefined { + return params[key] != null ? Number(params[key]) : def + } const bool = (key: string) => params[key] === true || params[key] === "1" || params[key] === "true" + /** A single-valued param as the one-element list the query filters take. */ + const strList = (key: string): string[] | undefined => { + const value = str(key) + return value === undefined ? undefined : [value] + } + + /** An `equals` attribute filter, present only when its key param is. */ + const equalsFilter = (keyParam: string, valueParam: string) => { + const key = str(keyParam) + return key === undefined ? undefined : [{ key, value: str(valueParam), mode: "equals" as const }] + } + // The service-free constraint is `CompiledQueryRowSchema`'s, pushed one level // up: a row schema decodes bytes off a socket, so it cannot ask for a service, // and a struct is service-free exactly when its fields are. @@ -195,7 +214,7 @@ export function compilePipeQuery( errorsOnly: bool("has_error"), minDurationMs: int("min_duration_ms"), maxDurationMs: int("max_duration_ms"), - environments: str("deployment_env") ? [str("deployment_env")!] : undefined, + environments: strList("deployment_env"), matchModes: { serviceName: str("service_match_mode") === "contains" ? "contains" : undefined, @@ -203,24 +222,11 @@ export function compilePipeQuery( deploymentEnv: str("deployment_env_match_mode") === "contains" ? "contains" : undefined, }, - attributeFilters: str("attribute_filter_key") - ? [ - { - key: str("attribute_filter_key")!, - value: str("attribute_filter_value"), - mode: "equals" as const, - }, - ] - : undefined, - resourceAttributeFilters: str("resource_filter_key") - ? [ - { - key: str("resource_filter_key")!, - value: str("resource_filter_value"), - mode: "equals" as const, - }, - ] - : undefined, + attributeFilters: equalsFilter("attribute_filter_key", "attribute_filter_value"), + resourceAttributeFilters: equalsFilter( + "resource_filter_key", + "resource_filter_value", + ), }), { orgId, startTime, endTime }, ), @@ -317,7 +323,7 @@ export function compilePipeQuery( cursor: str("cursor"), search: str("search"), limit: int("limit", 50), - environments: str("deployment_env") ? [str("deployment_env")!] : undefined, + environments: strList("deployment_env"), matchModes: str("deployment_env_match_mode") === "contains" ? { deploymentEnv: "contains" } @@ -338,7 +344,7 @@ export function compilePipeQuery( traceId: str("trace_id"), spanId: str("span_id"), search: str("search"), - environments: str("deployment_env") ? [str("deployment_env")!] : undefined, + environments: strList("deployment_env"), matchModes: str("deployment_env_match_mode") === "contains" ? { deploymentEnv: "contains" } @@ -354,7 +360,7 @@ export function compilePipeQuery( logsFacetsQuery({ serviceName: str("service"), severity: str("severity"), - environments: str("deployment_env") ? [str("deployment_env")!] : undefined, + environments: strList("deployment_env"), matchModes: str("deployment_env_match_mode") === "contains" ? { deploymentEnv: "contains" } @@ -399,7 +405,7 @@ export function compilePipeQuery( eraseType(compileUnion(servicesFacetsQuery(), { orgId, startTime, endTime })), ), Match.when("service_releases_timeline", () => { - const bucketSeconds = int("bucket_seconds", 300)! + const bucketSeconds = int("bucket_seconds", 300) return eraseType( compile( serviceReleasesTimelineQuery({ @@ -411,7 +417,7 @@ export function compilePipeQuery( ) }), Match.when("service_apdex_time_series", () => { - const bucketSeconds = int("bucket_seconds", 60)! + const bucketSeconds = int("bucket_seconds", 60) return eraseType( compile( serviceApdexTimeseriesQuery({ @@ -475,7 +481,7 @@ export function compilePipeQuery( fingerprintHash: String(params.fingerprint_hash), services: str("services")?.split(",").filter(Boolean), }), - { orgId, startTime, endTime, bucketSeconds: int("bucket_seconds", 3600)! }, + { orgId, startTime, endTime, bucketSeconds: int("bucket_seconds", 3600) }, ), ), ), @@ -539,7 +545,7 @@ export function compilePipeQuery( startTime, endTime, fingerprintHash: String(params.fingerprint_hash), - bucketSeconds: int("bucket_seconds", 3600)!, + bucketSeconds: int("bucket_seconds", 3600), }), ), ), @@ -686,7 +692,7 @@ export function compilePipeQuery( orgId, startTime, endTime, - bucketSeconds: int("bucket_seconds", 60)!, + bucketSeconds: int("bucket_seconds", 60), }), ) }), @@ -702,7 +708,7 @@ export function compilePipeQuery( compile( topOperationsQuery({ metric: (str("metric") ?? "count") as TracesMetric, - limit: int("limit", 20)!, + limit: int("limit", 20), }), { orgId, startTime, endTime, serviceName: str("service_name") ?? "" }, ), diff --git a/packages/query-engine/src/ch/queries/infra.ts b/packages/query-engine/src/ch/queries/infra.ts index 5cc62d45f..e198f8630 100644 --- a/packages/query-engine/src/ch/queries/infra.ts +++ b/packages/query-engine/src/ch/queries/infra.ts @@ -433,8 +433,11 @@ const podFilterConditions = ( excluded?.length ? CH.notInList(expr, excluded) : undefined, ] }), - CH.when(opts.workloadKind && opts.workloadName, () => - $.ResourceAttributes.get(workloadAttrKey(opts.workloadKind!)).eq(opts.workloadName!), + CH.when( + opts.workloadKind !== undefined && opts.workloadName !== undefined + ? { kind: opts.workloadKind, name: opts.workloadName } + : undefined, + (workload) => $.ResourceAttributes.get(workloadAttrKey(workload.kind)).eq(workload.name), ), ] diff --git a/packages/query-engine/src/ch/queries/logs.ts b/packages/query-engine/src/ch/queries/logs.ts index e63017bce..89d5916ae 100644 --- a/packages/query-engine/src/ch/queries/logs.ts +++ b/packages/query-engine/src/ch/queries/logs.ts @@ -16,7 +16,7 @@ import { deploymentEnvExpr } from "@maple/domain/tinybird/semconv-renames" import { buildAttrFilterCondition } from "../../traces-shared" import type { AttributeIndexMode, LogBodySearchMode } from "../../capabilities" import { edgeCondition, interiorConditions } from "./rollup-splice" -import { inclusionCondition, inclusionValues } from "./query-helpers" +import { inclusionCondition, inclusionValues, soleValue } from "./query-helpers" // Shared options @@ -126,9 +126,8 @@ function environmentCondition( ): CH.Condition | undefined { if (!opts.environments?.length) return undefined const envAttr = deploymentEnvExpr($.ResourceAttributes) - if (opts.matchModes?.deploymentEnv === "contains" && opts.environments.length === 1) { - return CH.positionCaseInsensitive(envAttr, CH.lit(opts.environments[0]!)).gt(0) - } + const needle = opts.matchModes?.deploymentEnv === "contains" ? soleValue(opts.environments) : undefined + if (needle !== undefined) return CH.positionCaseInsensitive(envAttr, CH.lit(needle)).gt(0) return CH.inList(envAttr, opts.environments) } @@ -138,9 +137,8 @@ function namespaceCondition( ): CH.Condition | undefined { if (!opts.namespaces?.length) return undefined const nsAttr = $.ResourceAttributes.get("service.namespace") - if (opts.matchModes?.serviceNamespace === "contains" && opts.namespaces.length === 1) { - return CH.positionCaseInsensitive(nsAttr, CH.lit(opts.namespaces[0]!)).gt(0) - } + const needle = opts.matchModes?.serviceNamespace === "contains" ? soleValue(opts.namespaces) : undefined + if (needle !== undefined) return CH.positionCaseInsensitive(nsAttr, CH.lit(needle)).gt(0) return CH.inList(nsAttr, opts.namespaces) } @@ -391,9 +389,8 @@ function buildLogsGroupNameExpr( if (groupByService) parts.push(CH.toString_($.ServiceName)) if (groupBySeverity) parts.push(CH.toString_($.SeverityText)) - if (parts.length === 1) { - return CH.coalesce(CH.nullIf(parts[0]!, ""), CH.lit("all")) - } + const onlyPart = soleValue(parts) + if (onlyPart !== undefined) return CH.coalesce(CH.nullIf(onlyPart, ""), CH.lit("all")) // Multi-part: filter empty strings before joining with separator const filtered = CH.arrayFilter("x -> x != ''", CH.arrayOf(...parts)) diff --git a/packages/query-engine/src/ch/queries/metrics.ts b/packages/query-engine/src/ch/queries/metrics.ts index 018ed5dab..3a04962d7 100644 --- a/packages/query-engine/src/ch/queries/metrics.ts +++ b/packages/query-engine/src/ch/queries/metrics.ts @@ -87,11 +87,10 @@ export function metricsTimeseriesQuery(opts: MetricsTimeseriesOpts) { : opts.groupByAttributeKey ? $.Attributes.get(opts.groupByAttributeKey) : CH.lit(""), - groupName: - opts.groupByAttributeKey || opts.groupByResourceAttributeKey - ? opts.groupByResourceAttributeKey - ? $.ResourceAttributes.get(opts.groupByResourceAttributeKey) - : $.Attributes.get(opts.groupByAttributeKey!) + groupName: opts.groupByResourceAttributeKey + ? $.ResourceAttributes.get(opts.groupByResourceAttributeKey) + : opts.groupByAttributeKey + ? $.Attributes.get(opts.groupByAttributeKey) : $.ServiceName, ...metricsSelectExprs($, isHistogram), })) @@ -393,11 +392,10 @@ export function metricsTimeseriesRateQuery( : opts.groupByAttributeKey ? $.Attributes.get(opts.groupByAttributeKey) : CH.lit(""), - groupName: - opts.groupByAttributeKey || opts.groupByResourceAttributeKey - ? opts.groupByResourceAttributeKey - ? $.resourceAttributeValue - : $.Attributes.get(opts.groupByAttributeKey!) + groupName: opts.groupByResourceAttributeKey + ? $.resourceAttributeValue + : opts.groupByAttributeKey + ? $.Attributes.get(opts.groupByAttributeKey) : $.ServiceName, rateValue: CH.sumIf($.delta.div($.time_delta), $.delta.gte(0).and($.time_delta.gt(0))), increaseValue: CH.sumIf($.delta, $.delta.gte(0)), diff --git a/packages/query-engine/src/ch/queries/query-helpers.ts b/packages/query-engine/src/ch/queries/query-helpers.ts index ee64b8d5a..142cc2a68 100644 --- a/packages/query-engine/src/ch/queries/query-helpers.ts +++ b/packages/query-engine/src/ch/queries/query-helpers.ts @@ -180,8 +180,19 @@ export const facetAttrExpr = ( ? deploymentEnvExpr(resourceAttributes) : resourceAttributes.get(attrKey) +/** + * The sole element of a one-element list, else `undefined`. + * + * Every "one value narrows to a substring/equality match, more than one is set + * membership" branch in this file asks the same question, and `length === 1` + * answers it for the reader without answering it for the type system. + */ +export const soleValue = (values: readonly A[]): A | undefined => + values.length === 1 ? values[0] : undefined + export function inclusionCondition(col: CH.Expr, values: readonly string[]): CH.Condition { - return values.length === 1 ? col.eq(values[0]!) : CH.inList(col, values) + const only = soleValue(values) + return only === undefined ? CH.inList(col, values) : col.eq(only) } /** @@ -194,9 +205,10 @@ export function inclusionCondition(col: CH.Expr, values: readonly string * multi-select means (there it is set membership, not fuzzy matching). */ export function matchOrIn(col: CH.Expr, values: readonly string[], contains: boolean): CH.Condition { - return contains && values.length === 1 - ? CH.positionCaseInsensitive(col, CH.lit(values[0]!)).gt(0) - : inclusionCondition(col, values) + const only = contains ? soleValue(values) : undefined + return only === undefined + ? inclusionCondition(col, values) + : CH.positionCaseInsensitive(col, CH.lit(only)).gt(0) } /** @@ -259,11 +271,12 @@ export function tracesBaseWhereConditions( $.SpanAttributes.get("http.route"), $.SpanAttributes.get("url.path"), ) - return mm?.spanName === "contains" && v.length === 1 - ? CH.positionCaseInsensitive($.SpanName, CH.lit(v[0]!)) + const needle = mm?.spanName === "contains" ? soleValue(v) : undefined + return needle === undefined + ? inclusionCondition($.SpanName, v).or(inclusionCondition(display, v)) + : CH.positionCaseInsensitive($.SpanName, CH.lit(needle)) .gt(0) - .or(CH.positionCaseInsensitive(display, CH.lit(v[0]!)).gt(0)) - : inclusionCondition($.SpanName, v).or(inclusionCondition(display, v)) + .or(CH.positionCaseInsensitive(display, CH.lit(needle)).gt(0)) }), CH.when(opts.statusCode, (v: string) => $.StatusCode.eq(v)), CH.whenTrue(!!opts.rootOnly, () => $.SpanKind.in_("Server", "Consumer").or($.ParentSpanId.eq(""))), @@ -508,11 +521,7 @@ export function tracesAggregatesWhereConditions( CH.when(services, (v: readonly string[]) => matchOrIn($.ServiceName, v, mm?.serviceName === "contains"), ), - CH.when(spanNames, (v: readonly string[]) => - mm?.spanName === "contains" && v.length === 1 - ? CH.positionCaseInsensitive($.SpanName, CH.lit(v[0]!)).gt(0) - : inclusionCondition($.SpanName, v), - ), + CH.when(spanNames, (v: readonly string[]) => matchOrIn($.SpanName, v, mm?.spanName === "contains")), CH.whenTrue(!!opts.rootOnly, () => $.IsEntryPoint.eq(1)), errorsOnlyCondition($.StatusCode, opts.errorsOnly), ] diff --git a/packages/query-engine/src/ch/queries/traces.ts b/packages/query-engine/src/ch/queries/traces.ts index ac6833397..bc683f0c6 100644 --- a/packages/query-engine/src/ch/queries/traces.ts +++ b/packages/query-engine/src/ch/queries/traces.ts @@ -35,6 +35,7 @@ import { tracesBaseWhereConditions, type TracesBaseWhereOpts, matchOrIn, + soleValue, } from "./query-helpers" /** @@ -227,7 +228,8 @@ function buildMvGroupNameExpr( } if (parts.length === 0) return CH.lit("all") - if (parts.length === 1) return CH.coalesce(CH.nullIf(parts[0]!, ""), CH.lit("all")) + const onlyPart = soleValue(parts) + if (onlyPart !== undefined) return CH.coalesce(CH.nullIf(onlyPart, ""), CH.lit("all")) const filtered = CH.arrayFilter("x -> x != ''", CH.arrayOf(...parts)) return CH.coalesce(CH.nullIf(CH.arrayStringConcat(filtered, " \u00b7 "), ""), CH.lit("all")) } @@ -256,7 +258,8 @@ function buildAggregatesGroupNameExpr( } if (parts.length === 0) return CH.lit("all") - if (parts.length === 1) return CH.coalesce(CH.nullIf(parts[0]!, ""), CH.lit("all")) + const onlyPart = soleValue(parts) + if (onlyPart !== undefined) return CH.coalesce(CH.nullIf(onlyPart, ""), CH.lit("all")) const filtered = CH.arrayFilter("x -> x != ''", CH.arrayOf(...parts)) return CH.coalesce(CH.nullIf(CH.arrayStringConcat(filtered, " \u00b7 "), ""), CH.lit("all")) } diff --git a/packages/query-engine/src/drain/drain.test.ts b/packages/query-engine/src/drain/drain.test.ts index 809d94c4c..fd982f61f 100644 --- a/packages/query-engine/src/drain/drain.test.ts +++ b/packages/query-engine/src/drain/drain.test.ts @@ -8,14 +8,7 @@ describe("Drain TemplateMiner", () => { tm.addLogMessage("user 99 logged in") tm.addLogMessage("user 4321 logged in") expect(tm.drain.clusterCount).toBe(1) - // SAFETY: the test inspects Drain's known internal store solely to verify cluster aggregation. - const [cluster] = Array.from( - ( - tm.drain as unknown as { - unlimitedStore: Map - } - ).unlimitedStore?.values() ?? [], - ) + const [cluster] = tm.drain.clusters() expect(cluster?.size).toBe(3) expect(cluster?.getTemplate()).toContain("logged") }) diff --git a/packages/query-engine/src/drain/drain.ts b/packages/query-engine/src/drain/drain.ts index e8eb6a445..6c92f18e5 100644 --- a/packages/query-engine/src/drain/drain.ts +++ b/packages/query-engine/src/drain/drain.ts @@ -2,6 +2,19 @@ import { LogCluster } from "./log-cluster" import { LruCache } from "./lru-cache" import { Node } from "./node" +/** + * The child node at `key`, created when absent. Replaces the `has`-then-`get!` + * pair the prefix-tree walk repeated nine times: the two calls could not tell + * the type system they were about the same key. + */ +const childAt = (node: Node, key: string): Node => { + const existing = node.keyToChildNode.get(key) + if (existing !== undefined) return existing + const created = new Node() + node.keyToChildNode.set(key, created) + return created +} + export class Drain { logClusterDepth: number private maxNodeDepth: number @@ -13,8 +26,12 @@ export class Drain { paramStr: string parametrizeNumericTokens: boolean - private unlimitedStore: Map | null - private limitedStore: LruCache | null + /** + * One store, not a nullable pair. `maxClusters` picks an LRU with eviction or + * an unbounded Map at construction and never changes it, so a union field + * says that where two mutually-exclusive nullable fields only implied it. + */ + private store: Map | LruCache clustersCounter: number constructor( @@ -41,50 +58,43 @@ export class Drain { this.parametrizeNumericTokens = parametrizeNumericTokens this.clustersCounter = 0 - if (maxClusters !== null) { - this.unlimitedStore = null - this.limitedStore = new LruCache(maxClusters) - } else { - this.unlimitedStore = new Map() - this.limitedStore = null - } + this.store = + maxClusters !== null ? new LruCache(maxClusters) : new Map() } get clusterCount(): number { - if (this.unlimitedStore) return this.unlimitedStore.size - return this.limitedStore!.size + return this.store.size + } + + /** Every live cluster, in whatever order the store holds them. */ + clusters(): LogCluster[] { + return this.store instanceof LruCache ? this.store.values() : [...this.store.values()] } getTotalClusterSize(): number { let total = 0 - if (this.unlimitedStore) { - for (const c of this.unlimitedStore.values()) total += c.size - } else { - for (const c of this.limitedStore!.values()) total += c.size - } + for (const cluster of this.clusters()) total += cluster.size return total } + /** Read without updating recency — only the LRU distinguishes the two. */ private clusterPeek(id: number): LogCluster | undefined { - if (this.unlimitedStore) return this.unlimitedStore.get(id) - return this.limitedStore!.peek(id) + return this.store instanceof LruCache ? this.store.peek(id) : this.store.get(id) } private clusterGet(id: number): LogCluster | undefined { - if (this.unlimitedStore) return this.unlimitedStore.get(id) - return this.limitedStore!.get(id) + return this.store.get(id) } private clusterContains(id: number): boolean { - if (this.unlimitedStore) return this.unlimitedStore.has(id) - return this.limitedStore!.has(id) + return this.store.has(id) } private clusterInsert(id: number, cluster: LogCluster): void { - if (this.unlimitedStore) { - this.unlimitedStore.set(id, cluster) + if (this.store instanceof LruCache) { + this.store.put(id, cluster) } else { - this.limitedStore!.put(id, cluster) + this.store.set(id, cluster) } } @@ -204,11 +214,7 @@ export class Drain { const tokenCount = templateTokens.length const tokenCountStr = String(tokenCount) - if (!this.rootNode.keyToChildNode.has(tokenCountStr)) { - this.rootNode.keyToChildNode.set(tokenCountStr, new Node()) - } - - let curNode = this.rootNode.keyToChildNode.get(tokenCountStr)! + let curNode = childAt(this.rootNode, tokenCountStr) if (tokenCount === 0) { curNode.clusterIds = [clusterId] @@ -224,32 +230,21 @@ export class Drain { break } - if (!curNode.keyToChildNode.has(token)) { - if (this.parametrizeNumericTokens && Drain.hasNumbers(token)) { - if (!curNode.keyToChildNode.has(this.paramStr)) { - curNode.keyToChildNode.set(this.paramStr, new Node()) - } - curNode = curNode.keyToChildNode.get(this.paramStr)! - } else if (curNode.keyToChildNode.has(this.paramStr)) { - if (curNode.keyToChildNode.size < this.maxChildren) { - curNode.keyToChildNode.set(token, new Node()) - curNode = curNode.keyToChildNode.get(token)! - } else { - curNode = curNode.keyToChildNode.get(this.paramStr)! - } - } else { - if (curNode.keyToChildNode.size + 1 < this.maxChildren) { - curNode.keyToChildNode.set(token, new Node()) - curNode = curNode.keyToChildNode.get(token)! - } else if (curNode.keyToChildNode.size + 1 === this.maxChildren) { - curNode.keyToChildNode.set(this.paramStr, new Node()) - curNode = curNode.keyToChildNode.get(this.paramStr)! - } else { - curNode = curNode.keyToChildNode.get(this.paramStr)! - } - } + if (curNode.keyToChildNode.has(token)) { + curNode = childAt(curNode, token) + } else if (this.parametrizeNumericTokens && Drain.hasNumbers(token)) { + curNode = childAt(curNode, this.paramStr) + } else if (curNode.keyToChildNode.has(this.paramStr)) { + curNode = + curNode.keyToChildNode.size < this.maxChildren + ? childAt(curNode, token) + : childAt(curNode, this.paramStr) + } else if (curNode.keyToChildNode.size + 1 < this.maxChildren) { + curNode = childAt(curNode, token) } else { - curNode = curNode.keyToChildNode.get(token)! + // At the cap: the wildcard child is created here and absorbs every + // token that would have pushed the fan-out past `maxChildren`. + curNode = childAt(curNode, this.paramStr) } currentDepth++ } @@ -258,8 +253,12 @@ export class Drain { addLogMessage(content: string): [LogCluster, string] { const contentTokens = this.getContentAsTokens(content) const matchClusterId = this.treeSearch(contentTokens, this.simTh, false) + // `fastMatch` only returns ids whose cluster it just read, so an id that no + // longer resolves means the store was evicted from under a synchronous + // call. Both cases are "no cluster to grow" — start a new one. + const existingCluster = matchClusterId === null ? undefined : this.clusterPeek(matchClusterId) - if (matchClusterId === null) { + if (existingCluster === undefined) { this.clustersCounter++ const clusterId = this.clustersCounter const cluster = new LogCluster(contentTokens, clusterId) @@ -268,7 +267,6 @@ export class Drain { return [cluster, "cluster_created"] } - const existingCluster = this.clusterPeek(matchClusterId)! const newTemplateTokens = this.createTemplate(contentTokens, existingCluster.logTemplateTokens) const updateType = @@ -281,7 +279,7 @@ export class Drain { existingCluster.size += 1 // Touch to update LRU ordering - this.clusterGet(matchClusterId) + this.clusterGet(existingCluster.clusterId) return [existingCluster, updateType] } diff --git a/packages/query-engine/src/drain/lru-cache.ts b/packages/query-engine/src/drain/lru-cache.ts index 28838ccaf..d05d2262f 100644 --- a/packages/query-engine/src/drain/lru-cache.ts +++ b/packages/query-engine/src/drain/lru-cache.ts @@ -1,3 +1,14 @@ +import { Schema } from "effect" + +/** + * A slot index that no longer holds an entry. The list, the free list, and the + * key map are maintained together, so reaching one is a bug in this file — not + * an input the caller can produce, and not something a caller could handle. + */ +class LruSlotError extends Schema.TaggedError()("@maple/query-engine/drain/LruSlotError", { + slot: Schema.Number, +}) {} + interface LruEntry { key: number value: V @@ -22,6 +33,18 @@ export class LruCache { this.capacity = capacity } + /** + * The entry at a slot the caller has already established is live — from + * `map`, from `head`/`tail`, or from a neighbour link. Replaces a bare `!`: + * the assertion is the same, but a broken invariant now names the slot + * instead of surfacing as `undefined` two frames later. + */ + private entryAt(slot: number): LruEntry { + const entry = this.entries[slot] + if (entry === null || entry === undefined) throw new LruSlotError({ slot }) + return entry + } + get size(): number { return this.map.size } @@ -49,8 +72,7 @@ export class LruCache { put(key: number, value: V): [number, V] | undefined { const existingSlot = this.map.get(key) if (existingSlot !== undefined) { - const entry = this.entries[existingSlot]! - entry.value = value + this.entryAt(existingSlot).value = value this.moveToHead(existingSlot) return undefined } @@ -68,7 +90,7 @@ export class LruCache { }) if (this.head !== null) { - this.entries[this.head]!.prev = slot + this.entryAt(this.head).prev = slot } this.head = slot if (this.tail === null) { @@ -84,7 +106,7 @@ export class LruCache { if (slot === undefined) return undefined this.map.delete(key) this.unlink(slot) - const entry = this.entries[slot]! + const entry = this.entryAt(slot) this.entries[slot] = null this.freeSlots.push(slot) return entry.value @@ -101,8 +123,8 @@ export class LruCache { } private allocSlot(entry: LruEntry): number { - if (this.freeSlots.length > 0) { - const slot = this.freeSlots.pop()! + const slot = this.freeSlots.pop() + if (slot !== undefined) { this.entries[slot] = entry return slot } @@ -111,14 +133,14 @@ export class LruCache { } private unlink(slot: number): void { - const entry = this.entries[slot]! + const entry = this.entryAt(slot) if (entry.prev !== null) { - this.entries[entry.prev]!.next = entry.next + this.entryAt(entry.prev).next = entry.next } else { this.head = entry.next } if (entry.next !== null) { - this.entries[entry.next]!.prev = entry.prev + this.entryAt(entry.next).prev = entry.prev } else { this.tail = entry.prev } @@ -127,11 +149,11 @@ export class LruCache { private moveToHead(slot: number): void { if (this.head === slot) return this.unlink(slot) - const entry = this.entries[slot]! + const entry = this.entryAt(slot) entry.prev = null entry.next = this.head if (this.head !== null) { - this.entries[this.head]!.prev = slot + this.entryAt(this.head).prev = slot } this.head = slot if (this.tail === null) { @@ -142,7 +164,7 @@ export class LruCache { private evictTail(): [number, V] | undefined { if (this.tail === null) return undefined const tailSlot = this.tail - const entry = this.entries[tailSlot]! + const entry = this.entryAt(tailSlot) const key = entry.key const value = entry.value this.map.delete(key) diff --git a/packages/query-engine/src/execution/backend.ts b/packages/query-engine/src/execution/backend.ts index a581275d2..7294c9d6a 100644 --- a/packages/query-engine/src/execution/backend.ts +++ b/packages/query-engine/src/execution/backend.ts @@ -9,6 +9,8 @@ * env-level self-hosted read endpoint * - `chdb` — the embedded chDB engine behind the local `maple` binary */ +import { Option, Schema } from "effect" + export type WarehouseBackendKind = "tinybird" | "tinybird-gateway" | "clickhouse" | "chdb" export interface TinybirdBackendConfig { @@ -51,14 +53,15 @@ export interface WarehouseTargetIdentity { * (host only), so a config carrying `https://api.tinybird.co` has to reduce to * the same string or the two sides land on different nodes again. */ +const decodeUrl = Schema.decodeUnknownOption(Schema.URLFromString) + const hostOf = (urlOrHost: string): string => { const trimmed = urlOrHost.trim() if (trimmed === "") return "" - try { - return new URL(trimmed.includes("://") ? trimmed : `https://${trimmed}`).host - } catch { - return trimmed - } + const parsed = decodeUrl(trimmed.includes("://") ? trimmed : `https://${trimmed}`) + // Not a URL even with a scheme bolted on — a bare identifier, or a config + // typo. Pass it through so the two sides still agree on the same node. + return Option.isSome(parsed) ? parsed.value.host : trimmed } export const warehouseTargetIdentity = (config: ResolvedWarehouseConfig): WarehouseTargetIdentity => diff --git a/packages/query-engine/src/execution/executor.ts b/packages/query-engine/src/execution/executor.ts index f97e979a6..855e01bf3 100644 --- a/packages/query-engine/src/execution/executor.ts +++ b/packages/query-engine/src/execution/executor.ts @@ -919,7 +919,10 @@ WHERE name = 'enable_full_text_index'`, capabilities: WarehouseCapabilities | undefined, ): Effect.Effect> => typeof compiled === "function" - ? Effect.orDie(compiled(capabilities!)) + ? // The capability-aware form is only reachable from the two methods that + // resolve capabilities first. Baseline is the conservative stand-in — it + // generates the widest SQL — if a caller ever reaches here without them. + Effect.orDie(compiled(capabilities ?? baselineWarehouseCapabilities())) : resolveCompiledQuery(compiled) const executeCompiledQuery = Effect.fn("WarehouseQueryService.executeCompiledQuery")(function* ( diff --git a/packages/query-engine/src/execution/managed-capabilities.ts b/packages/query-engine/src/execution/managed-capabilities.ts index a32537781..139732c6c 100644 --- a/packages/query-engine/src/execution/managed-capabilities.ts +++ b/packages/query-engine/src/execution/managed-capabilities.ts @@ -55,9 +55,9 @@ const parseTableDdl = (table: string, ddl: string): TableMetadata => { for (const line of body) { const entry = line.trim().replace(/,$/, "") if (entry === "") continue - const index = INDEX_LINE.exec(entry) - if (index) { - indexes.push({ table, name: index[1]!, type: index[3]!, expression: index[2]! }) + const [, indexName, indexExpression, indexType] = INDEX_LINE.exec(entry) ?? [] + if (indexName !== undefined && indexExpression !== undefined && indexType !== undefined) { + indexes.push({ table, name: indexName, type: indexType, expression: indexExpression }) continue } const name = entry.split(/\s+/)[0] diff --git a/packages/query-engine/src/formula-results.ts b/packages/query-engine/src/formula-results.ts index de47032f7..f9f01b47a 100644 --- a/packages/query-engine/src/formula-results.ts +++ b/packages/query-engine/src/formula-results.ts @@ -115,6 +115,11 @@ function compileFormula(expression: string): { const output: FormulaToken[] = [] const operatorStack: FormulaToken[] = [] + /** Move the top operator to the output; a no-op on an empty stack. */ + const emitTop = (): void => { + const top = operatorStack.pop() + if (top !== undefined) output.push(top) + } const identifiers = new Set() const precedence: Record = { "+": 1, @@ -148,8 +153,7 @@ function compileFormula(expression: string): { if (token.type === "rightParen") { let foundLeftParen = false - while (operatorStack.length > 0) { - const top = operatorStack.pop()! + for (let top = operatorStack.pop(); top !== undefined; top = operatorStack.pop()) { if (top.type === "leftParen") { foundLeftParen = true break @@ -158,9 +162,7 @@ function compileFormula(expression: string): { } // After the operand inside the parens has been emitted, any unary // operator that was waiting on it should be applied next. - while (operatorStack.length > 0 && operatorStack[operatorStack.length - 1].type === "unary") { - output.push(operatorStack.pop()!) - } + while (operatorStack.at(-1)?.type === "unary") emitTop() if (!foundLeftParen) { return { @@ -191,7 +193,7 @@ function compileFormula(expression: string): { while (operatorStack.length > 0) { const top = operatorStack[operatorStack.length - 1] if (top.type === "unary") { - output.push(operatorStack.pop()!) + emitTop() continue } if (top.type !== "operator") { @@ -202,7 +204,7 @@ function compileFormula(expression: string): { break } - output.push(operatorStack.pop()!) + emitTop() } operatorStack.push(token) @@ -210,8 +212,7 @@ function compileFormula(expression: string): { } } - while (operatorStack.length > 0) { - const top = operatorStack.pop()! + for (let top = operatorStack.pop(); top !== undefined; top = operatorStack.pop()) { if (top.type === "leftParen" || top.type === "rightParen") { return { rpn: [], @@ -263,7 +264,7 @@ function evaluateCompiledFormula( if (stack.length < 1) { return { value: null, error: "Invalid formula expression", reason: "other" } } - const operand = stack.pop()! + const operand = stack.pop() ?? 0 if (token.value === "u-") { stack.push(-operand) continue @@ -279,8 +280,10 @@ function evaluateCompiledFormula( return { value: null, error: "Invalid formula expression", reason: "other" } } - const right = stack.pop()! - const left = stack.pop()! + // Guarded by the `stack.length < 2` check above; `?? 0` is the arithmetic + // identity that keeps the expression total either way. + const right = stack.pop() ?? 0 + const left = stack.pop() ?? 0 if (token.value === "+") { stack.push(left + right) diff --git a/packages/query-engine/src/observability/error-detail.ts b/packages/query-engine/src/observability/error-detail.ts index de9e2ef78..a304c022f 100644 --- a/packages/query-engine/src/observability/error-detail.ts +++ b/packages/query-engine/src/observability/error-detail.ts @@ -130,7 +130,7 @@ export const errorDetail = Effect.fn("Observability.errorDetail")(function* (inp startTime: t.startTime, errorMessage: t.errorMessage ?? "", logs: pipe( - i < logsResults.length ? logsResults[i]!.data : [], + logsResults[i]?.data ?? [], Arr.take(5), Arr.map((l) => ({ timestamp: String(l.timestamp), diff --git a/packages/query-engine/src/route-rows.ts b/packages/query-engine/src/route-rows.ts index f3e28cc31..529821b79 100644 --- a/packages/query-engine/src/route-rows.ts +++ b/packages/query-engine/src/route-rows.ts @@ -14,7 +14,7 @@ * driver — the one input a shaper needs beyond its rows is the window's * duration, and the caller hands that in. */ -import { Schema } from "effect" +import { Option, Schema } from "effect" import { SpanId, TraceId } from "@maple/domain" import { parseWarehouseDateTime, warehouseDateTimeToIso } from "./datetime" @@ -324,17 +324,19 @@ export interface LogRow { const toTraceId = Schema.decodeSync(TraceId) const toSpanId = Schema.decodeSync(SpanId) +const decodeJsonObject = Schema.decodeUnknownOption( + Schema.fromJsonString(Schema.Record(Schema.String, Schema.Unknown)), +) + function parseAttributes(value: unknown): Record { if (typeof value !== "string" || value.length === 0) return {} - try { - const parsed: unknown = JSON.parse(value) - if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) return {} - // SAFETY: the warehouse serialises attribute maps as JSON objects of - // strings; a non-string value is a schema drift the readers tolerate. - return parsed as Record - } catch { - return {} - } + // `Schema.Record` admits only JSON objects, so an array, a scalar, and a + // malformed string all decode to `None` and degrade to an empty map. + const parsed = decodeJsonObject(value) + if (Option.isNone(parsed)) return {} + // SAFETY: the warehouse serialises attribute maps as JSON objects of + // strings; a non-string value is a schema drift the readers tolerate. + return parsed.value as Record } export function coerceLogRow(raw: Record): LogRow { diff --git a/packages/query-engine/src/runtime/cache-policy.ts b/packages/query-engine/src/runtime/cache-policy.ts index 4cf0657fd..121bb4124 100644 --- a/packages/query-engine/src/runtime/cache-policy.ts +++ b/packages/query-engine/src/runtime/cache-policy.ts @@ -16,14 +16,22 @@ export function makeDirectRouteCachePolicy( readonly version?: number } = {}, ): DirectRouteCachePolicy { - const ttlSeconds = Number.isFinite(options.ttlSeconds) - ? Math.max(1, Math.floor(options.ttlSeconds!)) - : DEFAULT_CACHE_SECONDS + // `Number.isFinite` narrows nothing on an optional field — bind the value and + // test the binding so the branch and the type agree. + const requestedTtl = options.ttlSeconds + const ttlSeconds = + requestedTtl !== undefined && Number.isFinite(requestedTtl) + ? Math.max(1, Math.floor(requestedTtl)) + : DEFAULT_CACHE_SECONDS const requestedSnap = options.snapWindowSeconds ?? ttlSeconds const snapWindowSeconds = Number.isFinite(requestedSnap) ? Math.min(3600, Math.max(1, Math.floor(requestedSnap))) : DEFAULT_CACHE_SECONDS - const version = Number.isFinite(options.version) ? Math.max(1, Math.floor(options.version!)) : 1 + const requestedVersion = options.version + const version = + requestedVersion !== undefined && Number.isFinite(requestedVersion) + ? Math.max(1, Math.floor(requestedVersion)) + : 1 return { version, ttlSeconds, snapWindowSeconds } } diff --git a/packages/query-engine/src/runtime/query-engine.ts b/packages/query-engine/src/runtime/query-engine.ts index 150ed5c38..277e54851 100644 --- a/packages/query-engine/src/runtime/query-engine.ts +++ b/packages/query-engine/src/runtime/query-engine.ts @@ -262,19 +262,19 @@ function traceServicePartitionWindow( * (`toJSONString` — Map columns can't survive an `argMin`). Decode defensively: * a malformed value degrades to an empty map, never a thrown defect. */ +const decodeProjectedAttributes = Schema.decodeUnknownOption( + Schema.fromJsonString(Schema.Record(Schema.String, Schema.Unknown)), +) + function parseProjectedAttributes(raw: unknown): Record { if (typeof raw !== "string" || raw.length === 0) return {} - try { - const parsed: unknown = JSON.parse(raw) - if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return {} - const out: Record = {} - for (const [key, value] of Object.entries(parsed)) { - if (typeof value === "string" && value.length > 0) out[key] = value - } - return out - } catch { - return {} + const parsed = decodeProjectedAttributes(raw) + if (Option.isNone(parsed)) return {} + const out: Record = {} + for (const [key, value] of Object.entries(parsed.value)) { + if (typeof value === "string" && value.length > 0) out[key] = value } + return out } function servicesForTraceRow( @@ -688,13 +688,15 @@ function groupTimeSeriesRows ({ bucket, - series: bucketMap.get(bucket)!, + series: bucketMap.get(bucket) ?? {}, })) } @@ -771,9 +775,11 @@ function groupAllMetricsTimeSeriesRows< } } + // Every ordered bucket was either seeded above or written while iterating + // rows; an empty series is the honest value for one that was neither. return bucketOrder.map((bucket) => ({ bucket, - series: bucketMap.get(bucket)!, + series: bucketMap.get(bucket) ?? {}, })) } @@ -1262,13 +1268,17 @@ export const makeQueryEngineExecute = (warehouse: QueryEn } const range = yield* validateExecute(request) + // Always a number. Only a timeseries query carries an explicit + // `bucketSeconds` or reports one on the span, but every timeseries branch + // below needs the value, and a `number | undefined` here meant each of them + // re-asserted the correlation the type system could not follow. + const isTimeseries = request.query.kind === "timeseries" const bucketSeconds = - request.query.kind === "timeseries" - ? (request.query.bucketSeconds ?? computeBucketSeconds(range.startMs, range.endMs)) - : undefined - if (bucketSeconds) yield* Effect.annotateCurrentSpan("query.bucketSeconds", bucketSeconds) + (isTimeseries ? request.query.bucketSeconds : undefined) ?? + computeBucketSeconds(range.startMs, range.endMs) + if (isTimeseries) yield* Effect.annotateCurrentSpan("query.bucketSeconds", bucketSeconds) - const fillOptions = bucketSeconds + const fillOptions = isTimeseries ? { startMs: range.startMs, endMs: range.endMs, @@ -1295,7 +1305,7 @@ export const makeQueryEngineExecute = (warehouse: QueryEn groupBy: tracesQuery.groupBy as string[] | undefined, apdexThresholdMs: tracesQuery.metric === "apdex" ? tracesQuery.apdexThresholdMs : undefined, - bucketSeconds: bucketSeconds!, + bucketSeconds, seriesLimit: tracesQuery.seriesLimit, overviewTiers, }), @@ -1303,7 +1313,7 @@ export const makeQueryEngineExecute = (warehouse: QueryEn orgId: tenant.orgId, startTime: request.startTime, endTime: request.endTime, - bucketSeconds: bucketSeconds!, + bucketSeconds, }, "tracesAllMetricsTimeseries", ) @@ -1359,14 +1369,14 @@ export const makeQueryEngineExecute = (warehouse: QueryEn groupBy: tracesQuery.groupBy as string[] | undefined, apdexThresholdMs: tracesQuery.metric === "apdex" ? tracesQuery.apdexThresholdMs : undefined, - bucketSeconds: bucketSeconds!, + bucketSeconds, seriesLimit: tracesQuery.seriesLimit, }), { orgId: tenant.orgId, startTime: request.startTime, endTime: request.endTime, - bucketSeconds: bucketSeconds!, + bucketSeconds, }, "tracesTimeseries", ) @@ -1387,7 +1397,7 @@ export const makeQueryEngineExecute = (warehouse: QueryEn warehouse, logsTimeseries, tenant, - toLogsTimeseriesInput(request.startTime, request.endTime, request.query, bucketSeconds!), + toLogsTimeseriesInput(request.startTime, request.endTime, request.query, bucketSeconds), ), logsTimeseries.id, ) @@ -1409,7 +1419,7 @@ export const makeQueryEngineExecute = (warehouse: QueryEn { startTime: request.startTime, endTime: request.endTime, - bucketSeconds: bucketSeconds!, + bucketSeconds, }, { value: "metricsTimeseries", rate: "metricsRateIncrease" }, ) diff --git a/packages/query-engine/src/runtime/raw-sql.ts b/packages/query-engine/src/runtime/raw-sql.ts index 3f5b10b80..c6de5eed8 100644 --- a/packages/query-engine/src/runtime/raw-sql.ts +++ b/packages/query-engine/src/runtime/raw-sql.ts @@ -1,4 +1,4 @@ -import { Effect } from "effect" +import { Effect, Option } from "effect" import { MAX_RAW_SQL_CELL_LENGTH, MAX_RAW_SQL_RESULT_BYTES, @@ -136,13 +136,11 @@ const rawSqlResultLimitError = (rows: ReadonlyArray>): s } } - let encoded: string - try { - encoded = JSON.stringify(row) ?? "null" - } catch { - return "Raw SQL results must be JSON serializable" - } - totalBytes += new TextEncoder().encode(encoded).byteLength + 1 + // A cyclic value or a BigInt cell throws out of `JSON.stringify` rather + // than returning, and the caller owes the user that as a 400. + const encoded = Effect.runSync(Effect.option(Effect.try(() => JSON.stringify(row) ?? "null"))) + if (Option.isNone(encoded)) return "Raw SQL results must be JSON serializable" + totalBytes += new TextEncoder().encode(encoded.value).byteLength + 1 if (totalBytes > MAX_RAW_SQL_RESULT_BYTES) { return `Raw SQL results may contain at most ${MAX_RAW_SQL_RESULT_BYTES} encoded bytes` } diff --git a/packages/query-engine/src/sql-catalog.ts b/packages/query-engine/src/sql-catalog.ts index 2915f899b..53782adb3 100644 --- a/packages/query-engine/src/sql-catalog.ts +++ b/packages/query-engine/src/sql-catalog.ts @@ -339,7 +339,7 @@ export function collectPipeCatalog(): ReadonlyArray { const entries: Array = [] for (const fixture of pipeFixtures) { - const variants = fixture.allCapabilities ? capabilityVariants : [capabilityVariants[0]!] + const variants = fixture.allCapabilities ? capabilityVariants : capabilityVariants.slice(0, 1) for (const variant of variants) { const params = { org_id: ORG_ID, @@ -860,7 +860,7 @@ export function collectQuerySpecCatalog(): ReadonlyArray { const tenant: QueryTenant = { orgId: OrgId.make(ORG_ID) } for (const fixture of querySpecFixtures) { - const variants = fixture.allCapabilities ? capabilityVariants : [capabilityVariants[0]!] + const variants = fixture.allCapabilities ? capabilityVariants : capabilityVariants.slice(0, 1) for (const variant of variants) { const { warehouse, captured } = makeCapturingWarehouse(variant.capabilities) const execute = makeQueryEngineExecute(warehouse) diff --git a/packages/query-engine/src/traces-shared.ts b/packages/query-engine/src/traces-shared.ts index 86c7b5ea5..40ab5b387 100644 --- a/packages/query-engine/src/traces-shared.ts +++ b/packages/query-engine/src/traces-shared.ts @@ -123,6 +123,18 @@ export function buildAttrFilterCondition( const colExpr: CH.Expr = coalescedMapGet(mapExpr, keys) const value = af.value ?? "" + /** + * OR one index prefilter per aliased key. `undefined` when there are no keys + * to prefilter on, in which case the exact predicate stands alone — an + * alias table never yields an empty list, but a prefilter over no keys is + * `has(…, NULL)` rather than a wider read. + */ + const orOverKeys = (make: (key: string) => CH.Condition): CH.Condition | undefined => + keys.reduce( + (acc, key) => (acc === undefined ? make(key) : acc.or(make(key))), + undefined, + ) + const positive = ((): CH.Condition => { if (af.mode === "exists") { // ClickHouse `Map` lookups return the value type's default (`''`) for a @@ -133,11 +145,8 @@ export function buildAttrFilterCondition( // makes `!exists` (the `NOT (...)` wrapper below) mean "absent or empty". const exact = anyMapContains(mapExpr, keys).and(colExpr.neq("")) if (af.negated || indexMode === "none") return exact - let candidate = CH.has(CH.mapKeys(mapExpr), CH.lit(keys[0]!)) - for (let i = 1; i < keys.length; i++) { - candidate = candidate.or(CH.has(CH.mapKeys(mapExpr), CH.lit(keys[i]!))) - } - return candidate.and(exact) + const candidate = orOverKeys((key) => CH.has(CH.mapKeys(mapExpr), CH.lit(key))) + return candidate === undefined ? exact : candidate.and(exact) } if (af.mode === "contains") { return CH.positionCaseInsensitive(colExpr, CH.lit(value)).gt(0) @@ -179,22 +188,19 @@ export function buildAttrFilterCondition( ResourceAttributes: "ResourceAttributeItems", } as const const items = CH.dynamicColumn>(itemColumnByMap[mapName]) - let candidate = CH.has(items, CH.concat(keys[0]!, CH.rawExpr("char(31)", T.string), value)) - for (let i = 1; i < keys.length; i++) { - candidate = candidate.or( - CH.has(items, CH.concat(keys[i]!, CH.rawExpr("char(31)", T.string), value)), - ) - } - return candidate.and(exact) + const candidate = orOverKeys((key) => + CH.has(items, CH.concat(key, CH.rawExpr("char(31)", T.string), value)), + ) + return candidate === undefined ? exact : candidate.and(exact) } // Bloom filters index keys and values independently. The original map // equality remains as exact confirmation, preventing cross-key matches. - let keyCandidate = CH.has(CH.mapKeys(mapExpr), CH.lit(keys[0]!)) - for (let i = 1; i < keys.length; i++) { - keyCandidate = keyCandidate.or(CH.has(CH.mapKeys(mapExpr), CH.lit(keys[i]!))) - } - return keyCandidate.and(CH.has(CH.mapValues(mapExpr), CH.lit(value))).and(exact) + const keyCandidate = orOverKeys((key) => CH.has(CH.mapKeys(mapExpr), CH.lit(key))) + const valueCandidate = CH.has(CH.mapValues(mapExpr), CH.lit(value)) + return keyCandidate === undefined + ? valueCandidate.and(exact) + : keyCandidate.and(valueCandidate).and(exact) })() return af.negated ? CH.not(positive) : positive diff --git a/packages/ui/src/components/attributes/attributes-table.tsx b/packages/ui/src/components/attributes/attributes-table.tsx index 844efc136..46d95dd1a 100644 --- a/packages/ui/src/components/attributes/attributes-table.tsx +++ b/packages/ui/src/components/attributes/attributes-table.tsx @@ -1,6 +1,9 @@ // BOUNDARY: This module intentionally carries opaque values; callers decode them before domain use. "use client" +import { Option } from "effect" + +import { trySync } from "../../lib/try-sync" import { ChevronRightIcon } from "../icons" import { cn } from "../../lib/utils" import { useCopy } from "../../hooks/use-copy" @@ -63,11 +66,7 @@ export function CopyableValue({ export function tryParseJson(value: string): unknown | null { const trimmed = value.trimStart() if (trimmed[0] !== "{" && trimmed[0] !== "[") return null - try { - return JSON.parse(value) - } catch { - return null - } + return Option.getOrNull(trySync(() => JSON.parse(value))) } export function AttributeRow({ diff --git a/packages/ui/src/components/filters/range-filter-section.tsx b/packages/ui/src/components/filters/range-filter-section.tsx index 629c6bba7..294dee4ba 100644 --- a/packages/ui/src/components/filters/range-filter-section.tsx +++ b/packages/ui/src/components/filters/range-filter-section.tsx @@ -319,6 +319,12 @@ function RangeHistogram({ const [hoverIndex, setHoverIndex] = React.useState(undefined) const [dragStart, setDragStart] = React.useState(undefined) + // The caller only renders this for `histogram.length > 1`; establishing the + // ends once is what lets every read below be a plain property access. + const first = buckets[0] + const last = buckets.at(-1) + if (first === undefined || last === undefined) return null + const peak = Math.max(...buckets.map((b) => b.count), 1) const total = buckets.reduce((sum, b) => sum + b.count, 0) @@ -348,12 +354,13 @@ function RangeHistogram({ setDragStart(undefined) // A click picks a floor and leaves the top open — "at least this long" is // the dominant intent, and a single bucket is too narrow to be useful. + const bottom = buckets[lo] ?? first if (lo === hi) { - onSelect(buckets[lo]!.from, undefined) + onSelect(bottom.from, undefined) return } - const top = buckets[hi]! - onSelect(buckets[lo]!.from, top.unbounded ? undefined : top.to) + const top = buckets[hi] ?? last + onSelect(bottom.from, top.unbounded ? undefined : top.to) } // While dragging, preview the pending selection instead of the applied one. @@ -375,7 +382,6 @@ function RangeHistogram({ const hasSelection = minValue !== undefined || maxValue !== undefined || previewLo !== undefined const hovered = hoverIndex !== undefined ? buckets[hoverIndex] : undefined - const last = buckets[buckets.length - 1]! return (
@@ -400,7 +406,7 @@ function RangeHistogram({
- {formatValue(buckets[0]!.from, unit)} - {formatValue(buckets[Math.floor(buckets.length / 2)]!.from, unit)} + {formatValue(first.from, unit)} + {formatValue((buckets[Math.floor(buckets.length / 2)] ?? first).from, unit)} {last.unbounded ? `${formatValue(last.from, unit)}+` : formatValue(last.to, unit)} @@ -465,7 +471,9 @@ export function parseRange(text: string, unit: RangeUnit): number | undefined { if (!/^(\d+(\.\d+)?\s*(ms|s|m|h)\s*)+$/.test(trimmed)) return undefined let totalMs = 0 for (const [, amount, suffix] of trimmed.matchAll(/(\d+(?:\.\d+)?)\s*(ms|s|m|h)/g)) { - totalMs += Number(amount) * UNIT_MS[suffix!]! + const multiplier = suffix === undefined ? undefined : UNIT_MS[suffix] + if (multiplier === undefined) continue + totalMs += Number(amount) * multiplier } return unit === "ms" ? totalMs : totalMs / 1000 } diff --git a/packages/ui/src/components/plot/plot-frame.tsx b/packages/ui/src/components/plot/plot-frame.tsx index d6b30c3c1..c6fcfa37f 100644 --- a/packages/ui/src/components/plot/plot-frame.tsx +++ b/packages/ui/src/components/plot/plot-frame.tsx @@ -1,4 +1,7 @@ /// +import { Option } from "effect" + +import { trySync } from "../../lib/try-sync" import { CanvasChart, Chart as SvgChart } from "@tanstack/charts/react/tooltip" import type { ChartTooltipBodyRenderContext } from "@tanstack/charts/react/tooltip" import type { @@ -471,11 +474,11 @@ function supportsCanvas2d(): boolean { // server output anyway. return false } - try { - canvasSupport = document.createElement("canvas").getContext("2d") != null - } catch { - canvasSupport = false - } + // A hardened browser throws from `getContext` rather than returning null. + canvasSupport = Option.getOrElse( + trySync(() => document.createElement("canvas").getContext("2d") != null), + () => false, + ) return canvasSupport } diff --git a/packages/ui/src/components/traces/flamegraph.tsx b/packages/ui/src/components/traces/flamegraph.tsx index 25128f915..6421b2347 100644 --- a/packages/ui/src/components/traces/flamegraph.tsx +++ b/packages/ui/src/components/traces/flamegraph.tsx @@ -84,10 +84,8 @@ function assignLanes(bars: FlamegraphBar[]): { bars: FlamegraphBar[]; totalLanes } let laneOffset = 0 - const depths = Array.from(byDepth.keys()).sort((a, b) => a - b) - for (const depth of depths) { - const group = byDepth.get(depth)! + for (const [depth, group] of [...byDepth.entries()].sort(([a], [b]) => a - b)) { group.sort((a, b) => a.leftPercent - b.leftPercent) const lanes: number[] = [] diff --git a/packages/ui/src/components/ui/copy-button.tsx b/packages/ui/src/components/ui/copy-button.tsx index aa5c7b768..b964f7227 100644 --- a/packages/ui/src/components/ui/copy-button.tsx +++ b/packages/ui/src/components/ui/copy-button.tsx @@ -1,5 +1,8 @@ "use client" +import { Option } from "effect" + +import { trySync } from "../../lib/try-sync" import * as React from "react" import { useCopy, type CopyStatus, type UseCopyOptions } from "../../hooks/use-copy" @@ -224,11 +227,7 @@ export interface CopyButtonProps * `JSON.stringify`, say) escape as an uncaught click handler error. */ function resolveValue(value: string | (() => string)): string | null { if (typeof value !== "function") return value - try { - return value() - } catch { - return null - } + return Option.getOrNull(trySync(value)) } /** @@ -308,7 +307,9 @@ export function CopyButton({ return ( - {copyTooltipText(status, label, { copiedLabel, errorLabel: errorText })} + + {copyTooltipText(status, label, { copiedLabel, errorLabel: errorText })} + ) } diff --git a/packages/ui/src/hooks/use-copy.tsx b/packages/ui/src/hooks/use-copy.tsx index 396d388d2..83f81e222 100644 --- a/packages/ui/src/hooks/use-copy.tsx +++ b/packages/ui/src/hooks/use-copy.tsx @@ -2,14 +2,28 @@ "use client" import * as React from "react" +import { Effect, Option, Result, Schema } from "effect" import { toastManager } from "../components/ui/toast" import { writeClipboardFallback } from "../lib/clipboard" +import { trySync } from "../lib/try-sync" import { useClipboard } from "./use-clipboard" import { useMountEffect } from "./use-mount-effect" export type CopyStatus = "idle" | "copied" | "error" +/** The platform clipboard rejected — insecure origin, denied permission, unfocused document. */ +class ClipboardWriteError extends Schema.TaggedError()( + "@maple/ui/hooks/ClipboardWriteError", + { cause: Schema.Defect() }, +) {} + +/** `copy()` was handed an empty or absent value; nothing reached the clipboard. */ +class NothingToCopyError extends Schema.TaggedError()( + "@maple/ui/hooks/NothingToCopyError", + {}, +) {} + export interface UseCopyOptions { /** Human label for the thing being copied, e.g. "Trace ID". Drives toast copy. */ label?: string @@ -80,21 +94,30 @@ export function useCopy({ const copy = React.useCallback( async (text: string | null | undefined): Promise => { let ok = false - let reason: Error | null = null + let reason: ClipboardWriteError | NothingToCopyError | null = null if (!text) { - reason = new Error("Nothing to copy") + reason = new NothingToCopyError() } else { - try { - await clipboard.copy(text) + // The platform clipboard rejects on an insecure origin, a denied + // permission, and an unfocused document; the hidden-textarea fallback + // covers all three, and can itself throw in a sandboxed frame. + const written = await Effect.runPromise( + Effect.result( + Effect.tryPromise({ + try: () => clipboard.copy(text), + catch: (cause) => new ClipboardWriteError({ cause }), + }), + ), + ) + if (Result.isSuccess(written)) { ok = true - } catch (error) { - reason = error instanceof Error ? error : new Error(String(error)) - try { - ok = writeClipboardFallback(text) - } catch { - ok = false - } + } else { + reason = written.failure + ok = Option.getOrElse( + trySync(() => writeClipboardFallback(text)), + () => false, + ) } } diff --git a/packages/ui/src/hooks/use-section-collapse.ts b/packages/ui/src/hooks/use-section-collapse.ts index 526640632..909e8838d 100644 --- a/packages/ui/src/hooks/use-section-collapse.ts +++ b/packages/ui/src/hooks/use-section-collapse.ts @@ -3,6 +3,8 @@ import * as React from "react" import { Option, Schema } from "effect" +import { readLocalStorage, writeLocalStorage } from "../lib/local-storage" + /** * Remembered open/closed state for one collapsible filter section. * @@ -25,23 +27,14 @@ type SectionState = typeof SectionState.Type const decodeSectionState = Schema.decodeUnknownOption(Schema.fromJsonString(SectionState)) function read(): SectionState { - try { - const raw = localStorage.getItem(STORAGE_KEY) - if (raw === null) return {} - return Option.getOrElse(decodeSectionState(raw), (): SectionState => ({})) - } catch { - // localStorage unavailable (private mode / SSR) — the preference is a - // nicety, so fall back to defaults rather than throwing. - return {} - } + // Unavailable storage and an unreadable entry are the same answer here: no + // preference, so every section falls back to the caller's default. + const stored = Option.flatMap(readLocalStorage(STORAGE_KEY), decodeSectionState) + return Option.getOrElse(stored, (): SectionState => ({})) } function write(key: string, open: boolean): void { - try { - localStorage.setItem(STORAGE_KEY, JSON.stringify({ ...read(), [key]: open })) - } catch { - // Quota or unavailable — the session keeps working, it just won't remember. - } + writeLocalStorage(STORAGE_KEY, JSON.stringify({ ...read(), [key]: open })) } /** diff --git a/packages/ui/src/hooks/use-theme.ts b/packages/ui/src/hooks/use-theme.ts index 42479be84..76c312454 100644 --- a/packages/ui/src/hooks/use-theme.ts +++ b/packages/ui/src/hooks/use-theme.ts @@ -1,6 +1,9 @@ "use client" import { useSyncExternalStore } from "react" +import { Option } from "effect" + +import { readLocalStorage, writeLocalStorage } from "../lib/local-storage" export type Theme = "light" | "dark" @@ -19,13 +22,13 @@ function readInitialTheme(): Theme { if (root.classList.contains("light")) return "light" if (root.classList.contains("dark")) return "dark" } - try { - const stored = localStorage.getItem(STORAGE_KEY) - if (stored === "light" || stored === "dark") return stored - } catch { - // localStorage unavailable (private mode / non-browser) — use the default. - } - return DEFAULT_THEME + // localStorage may be unavailable (private mode / non-browser), and a + // hand-edited entry may be neither theme — both fall through to the default. + const stored = Option.filter( + readLocalStorage(STORAGE_KEY), + (value): value is Theme => value === "light" || value === "dark", + ) + return Option.getOrElse(stored, () => DEFAULT_THEME) } let current: Theme = readInitialTheme() @@ -73,11 +76,7 @@ function getServerSnapshot(): Theme { /** Set the active theme, persist it, and apply the `light`/`dark` class to . */ export function setTheme(theme: Theme): void { current = theme - try { - localStorage.setItem(STORAGE_KEY, theme) - } catch { - // Ignore persistence failures. - } + writeLocalStorage(STORAGE_KEY, theme) applyTheme(theme) notify() } diff --git a/packages/ui/src/lib/clipboard.ts b/packages/ui/src/lib/clipboard.ts index 1752b8abb..373be7e5c 100644 --- a/packages/ui/src/lib/clipboard.ts +++ b/packages/ui/src/lib/clipboard.ts @@ -4,6 +4,10 @@ * insecure-origin fallback instead of keeping their own copy of it. */ +import { Option } from "effect" + +import { tryPromise, trySync } from "./try-sync" + /** * Last-resort clipboard write for insecure origins and embedded contexts where * `navigator.clipboard` is missing or rejects. Restores the user's selection so @@ -25,12 +29,12 @@ export function writeClipboardFallback(text: string): boolean { const previous = selection && selection.rangeCount > 0 ? selection.getRangeAt(0) : null area.select() - let ok = false - try { - ok = document.execCommand("copy") - } catch { - ok = false - } + // `execCommand` throws outright in a sandboxed frame rather than returning + // false, so an unavailable command and a refused one are one answer. + const ok = Option.getOrElse( + trySync(() => document.execCommand("copy")), + () => false, + ) document.body.removeChild(area) if (selection && previous) { @@ -49,14 +53,12 @@ export function writeClipboardFallback(text: string): boolean { export async function writeClipboardText(text: string): Promise { if (!text) return false - try { - if (navigator.clipboard?.writeText) { - await navigator.clipboard.writeText(text) - return true - } - } catch { - // fall through to the legacy path + if (navigator.clipboard?.writeText) { + const written = await tryPromise(() => navigator.clipboard.writeText(text)) + if (Option.isSome(written)) return true } + // The API is missing, or it rejected — an insecure origin, a denied + // permission, a document that was not focused. The textarea covers all three. return writeClipboardFallback(text) } diff --git a/packages/ui/src/lib/error-body.ts b/packages/ui/src/lib/error-body.ts index 0285457f5..584363669 100644 --- a/packages/ui/src/lib/error-body.ts +++ b/packages/ui/src/lib/error-body.ts @@ -8,6 +8,10 @@ * shape here — pure, testable, no React — and let the component paint it. */ +import { Option } from "effect" + +import { trySync } from "./try-sync" + export type ErrorBodyFormat = "json" | "text" export interface ErrorBody { @@ -28,14 +32,13 @@ export function parseErrorBody(message: string): ErrorBody { const trimmed = message.trim() if (looksLikeJson(trimmed)) { - try { - // The delimiter check already guarantees an object or an array — the - // JSON grammar admits nothing else between those braces — so a parse - // that returns is a parse worth pretty-printing. - const parsed: unknown = JSON.parse(trimmed) - return { format: "json", full: JSON.stringify(parsed, null, 2) } - } catch { - // Truncated or otherwise malformed — fall through to text. + // The delimiter check already guarantees an object or an array — the JSON + // grammar admits nothing else between those braces — so a parse that + // succeeds is a parse worth pretty-printing. A truncated or otherwise + // malformed body decodes to `None` and falls through to text. + const parsed = trySync(() => JSON.parse(trimmed)) + if (Option.isSome(parsed)) { + return { format: "json", full: JSON.stringify(parsed.value, null, 2) } } } diff --git a/packages/ui/src/lib/gen-ai.ts b/packages/ui/src/lib/gen-ai.ts index 29a81eccd..134d531db 100644 --- a/packages/ui/src/lib/gen-ai.ts +++ b/packages/ui/src/lib/gen-ai.ts @@ -10,6 +10,9 @@ // plus the legacy aliases real spans still emit. The labels live here because a // label is a UI decision the convention does not make. +import { Option } from "effect" + +import { trySync } from "./try-sync" import { formatDuration } from "./format" const GEN_AI_PREFIX = "gen_ai." @@ -327,12 +330,7 @@ function formatValue(key: string, rawValue: string): string { /** An array of primitives, else null — an array of objects stays JSON. */ function parseFlatArray(rawValue: string): string[] | null { if (rawValue.trimStart()[0] !== "[") return null - let parsed: unknown - try { - parsed = JSON.parse(rawValue) - } catch { - return null - } + const parsed = Option.getOrNull(trySync(() => JSON.parse(rawValue))) if (!Array.isArray(parsed)) return null if (!parsed.every((item) => item === null || typeof item !== "object")) return null return parsed.map((item) => String(item)) diff --git a/packages/ui/src/lib/http.ts b/packages/ui/src/lib/http.ts index 661d303f2..c334d03d5 100644 --- a/packages/ui/src/lib/http.ts +++ b/packages/ui/src/lib/http.ts @@ -69,10 +69,10 @@ const parseSpanName = (name: string): Option.Option => { }), ), Match.when( - (p) => p.length === 1 && isHttpMethod(p[0]!), + (p): p is [string] => p.length === 1 && isHttpMethod(p[0]), ([method]) => Option.some({ - method: method!.toUpperCase(), + method: method.toUpperCase(), routeHint: Option.none(), }), ), diff --git a/packages/ui/src/lib/local-storage.ts b/packages/ui/src/lib/local-storage.ts new file mode 100644 index 000000000..17aa500e5 --- /dev/null +++ b/packages/ui/src/lib/local-storage.ts @@ -0,0 +1,21 @@ +import { Effect, Option } from "effect" +import { trySync } from "./try-sync" + +/** + * `localStorage` behind `Option`. + * + * Every access throws outright — not returns null — in private mode, in a + * sandboxed iframe, and wherever a browser has site data blocked, so both the + * read and the write need the same guard. Preferences stored here are always a + * nicety; a caller that cannot read one falls back to its default. + */ +export const readLocalStorage = (key: string): Option.Option => + Option.flatMapNullishOr( + trySync(() => localStorage.getItem(key)), + (value) => value, + ) + +/** Best-effort write. A quota or availability failure is silently dropped. */ +export const writeLocalStorage = (key: string, value: string): void => { + Effect.runSync(Effect.ignore(Effect.try(() => localStorage.setItem(key, value)))) +} diff --git a/packages/ui/src/lib/replay-format.ts b/packages/ui/src/lib/replay-format.ts index 25bf48b7c..3f9278553 100644 --- a/packages/ui/src/lib/replay-format.ts +++ b/packages/ui/src/lib/replay-format.ts @@ -3,6 +3,10 @@ // two can't drift. Warehouse-coupled helpers (partition windows) stay in the // web app: this package doesn't depend on @maple/query-engine. +import { Option, Schema } from "effect" + +const decodeUrl = Schema.decodeUnknownOption(Schema.URLFromString) + /** * `6h 12m` / `1m 23s` / `45s`, or `—` for missing/zero durations — a replay * with no measurable duration is unmeasured, not instantaneous. @@ -14,6 +18,7 @@ * Minutes roll over at an hour: agent sessions that wait on a human run for * hours, and "360m 0s" is not a duration anyone reads as six. */ + export function formatSessionDuration(ms: number | null): string { if (ms == null || ms <= 0) return "—" const totalSeconds = Math.round(ms / 1000) @@ -34,15 +39,13 @@ export function formatClock(ms: number): string { /** Host + path for compact URL display; returns the raw input if unparseable. */ export function hostFromUrl(url: string): string { - try { - const u = new URL(url) - return `${u.host}${u.pathname === "/" ? "" : u.pathname}` - } catch { - return url - } + const parsed = decodeUrl(url) + if (Option.isNone(parsed)) return url + const { host, pathname } = parsed.value + return `${host}${pathname === "/" ? "" : pathname}` } -const AVATAR_GRADIENTS = [ +const AVATAR_GRADIENTS: readonly [string, ...string[]] = [ "from-rose-500/80 to-orange-400/80", "from-violet-500/80 to-fuchsia-400/80", "from-sky-500/80 to-cyan-400/80", @@ -55,7 +58,7 @@ const AVATAR_GRADIENTS = [ export function gradientFor(seed: string): string { let hash = 0 for (let i = 0; i < seed.length; i++) hash = (hash * 31 + seed.charCodeAt(i)) >>> 0 - return AVATAR_GRADIENTS[hash % AVATAR_GRADIENTS.length]! + return AVATAR_GRADIENTS[hash % AVATAR_GRADIENTS.length] ?? AVATAR_GRADIENTS[0] } /** `true` for handheld device-type strings as reported by the browser SDK. */ diff --git a/packages/ui/src/lib/sanitizers.ts b/packages/ui/src/lib/sanitizers.ts index 56f941dce..a5fb9904f 100644 --- a/packages/ui/src/lib/sanitizers.ts +++ b/packages/ui/src/lib/sanitizers.ts @@ -5,6 +5,10 @@ * fall back, etc.) rather than ever rendering an unvetted string. */ +import { Option, Schema } from "effect" + +const decodeUrl = Schema.decodeUnknownOption(Schema.URLFromString) + const CSS_COLOR_RE = /^(?:#(?:[0-9a-f]{3}|[0-9a-f]{4}|[0-9a-f]{6}|[0-9a-f]{8})|(?:rgba?|hsla?|hwb|lab|lch|oklab|oklch|color)\([^()]*\)|var\(--[a-z0-9_-]+(?:,[^()]*)?\)|currentColor|transparent|inherit|initial|unset|[a-z]+)$/i @@ -50,12 +54,11 @@ export const validateUrlScheme = (raw: string | undefined | null): string | null if (trimmed.length === 0) return null if (trimmed.startsWith("//")) return null if (trimmed.startsWith("/") && !trimmed.startsWith("//")) return trimmed - try { - const parsed = new URL(trimmed) - return ALLOWED_HREF_SCHEMES.has(parsed.protocol) ? trimmed : null - } catch { - return null - } + return Option.getOrNull( + Option.filter(decodeUrl(trimmed), (parsed) => ALLOWED_HREF_SCHEMES.has(parsed.protocol)).pipe( + Option.map(() => trimmed), + ), + ) } /** diff --git a/packages/ui/src/lib/span-tree.ts b/packages/ui/src/lib/span-tree.ts index 5f1933099..c68d0a9a9 100644 --- a/packages/ui/src/lib/span-tree.ts +++ b/packages/ui/src/lib/span-tree.ts @@ -1,6 +1,7 @@ -import { Schema } from "effect" +import { Option, Schema } from "effect" import { TraceId, SpanId } from "@maple/domain" import type { Span, SpanNode } from "./types" +import { trySync } from "./try-sync" const toTraceId = Schema.decodeSync(TraceId) const toSpanId = Schema.decodeSync(SpanId) @@ -27,12 +28,11 @@ export interface SpanHierarchyRow { /** JSON-parse an attribute column, tolerating null/empty/garbage. */ export function parseAttributes(value: string | null | undefined): Record { if (!value) return {} - try { - const parsed = JSON.parse(value) - return parsed && typeof parsed === "object" ? (parsed as Record) : {} - } catch { - return {} - } + const parsed = Option.filter( + trySync(() => JSON.parse(value)), + (decoded): decoded is Record => decoded !== null && typeof decoded === "object", + ) + return Option.getOrElse(parsed, (): Record => ({})) } /** Map a raw hierarchy row into a branded `Span`. */ diff --git a/packages/ui/src/lib/try-sync.ts b/packages/ui/src/lib/try-sync.ts new file mode 100644 index 000000000..7888b5a1e --- /dev/null +++ b/packages/ui/src/lib/try-sync.ts @@ -0,0 +1,25 @@ +import { Effect, Option } from "effect" + +/** + * A throwing synchronous call as a total `Option`. + * + * The DOM is full of calls that throw rather than return a failure — `JSON.parse` + * on a truncated body, `localStorage` in private mode, `execCommand` in a + * sandboxed frame, a `getContext("2d")` a hardened browser refuses. `Effect.try` + * moves the throw into the error channel and `Effect.option` discards it, so the + * caller branches on a value the type system can see instead of on control flow + * it cannot. + * + * Costs ~0.5µs per call against ~0.05µs for a bare `try`/`catch`. That is fine + * everywhere it is used here; a genuinely hot loop should hoist the decision out + * rather than reach back for a `catch` block. + */ +export const trySync = (thunk: () => A): Option.Option => + Effect.runSync(Effect.option(Effect.try(thunk))) + +/** + * The async twin, for a promise that rejects rather than a call that throws. + * Resolves to `None` on rejection. + */ +export const tryPromise = (thunk: () => Promise): Promise> => + Effect.runPromise(Effect.option(Effect.tryPromise(thunk))) diff --git a/packages/widgets/src/chart/static-chart.ts b/packages/widgets/src/chart/static-chart.ts index c25d32263..bce05c26f 100644 --- a/packages/widgets/src/chart/static-chart.ts +++ b/packages/widgets/src/chart/static-chart.ts @@ -192,8 +192,10 @@ export function downsample( ): ReadonlyArray { if (points.length <= max || max < 3) return points const sorted = [...points].sort((a, b) => a[0] - b[0]) - const first = sorted[0]! - const last = sorted[sorted.length - 1]! + const first = sorted[0] + const last = sorted.at(-1) + // `points.length <= max` returned above and `max >= 3`, so both ends exist. + if (first === undefined || last === undefined) return points const inner = sorted.slice(1, -1) const buckets = max - 2 const size = Math.ceil(inner.length / buckets) @@ -220,7 +222,11 @@ export function downsample( */ export function renderPlotSvg(spec: StaticChartSpec): PlotRender { const points = [...spec.points].sort((a, b) => a[0] - b[0]) - if (points.length === 0) throw new Error("renderPlotSvg needs at least one data point.") + const firstPoint = points[0] + const lastPoint = points.at(-1) + if (firstPoint === undefined || lastPoint === undefined) { + throw new Error("renderPlotSvg needs at least one data point.") + } const threshold = spec.threshold ?? null const breachSide = spec.breachSide ?? "none" @@ -230,10 +236,11 @@ export function renderPlotSvg(spec: StaticChartSpec): PlotRender { // chart whose breach line sits off the top edge is worse than no chart. const domain = threshold === null ? values : [...values, threshold] const ticks = niceTicks(Math.min(...domain), Math.max(...domain)) - const yMin = ticks[0]! - const yMax = ticks[ticks.length - 1]! - const tMin = points[0]![0] - const tMax = points[points.length - 1]![0] + // `niceTicks` always returns at least a `[min, max]` pair. + const yMin = ticks[0] ?? 0 + const yMax = ticks.at(-1) ?? yMin + const tMin = firstPoint[0] + const tMax = lastPoint[0] const tRange = Math.max(1, tMax - tMin) const plotW = PLOT_WIDTH - PAD * 2 @@ -311,7 +318,7 @@ export function renderPlotSvg(spec: StaticChartSpec): PlotRender { ) // The latest value gets a dot with a 2px surface ring; its *number* is a // label the caller draws, because this SVG cannot. - const [lt, lv] = points[points.length - 1]! + const [lt, lv] = lastPoint parts.push( ``, ) @@ -330,7 +337,7 @@ export function renderPlotSvg(spec: StaticChartSpec): PlotRender { return { svg: parts.join("\n"), title: spec.title, - latest: formatValue(points[points.length - 1]![1], spec.unit), + latest: formatValue(lastPoint[1], spec.unit), threshold: threshold === null ? null diff --git a/scripts/oxlint-plugins/maple.mjs b/scripts/oxlint-plugins/maple.mjs index a89be8467..292fcf637 100644 --- a/scripts/oxlint-plugins/maple.mjs +++ b/scripts/oxlint-plugins/maple.mjs @@ -9,6 +9,9 @@ * site instead is both noise and, where a request value reaches a param, a 500 * where the route owed a 400. * + * `no-try-catch` is the syntax half of the repo's Effect-errors convention: oxlint + * has no `no-restricted-syntax`, so banning a statement kind takes a plugin rule. + * * `no-record-string-any` exists as its own rule (rather than leaning on * `typescript/no-explicit-any`, which flags the inner `any` anyway) so the worst * offender — an open key set whose values are also unchecked — can sit at `error` @@ -200,11 +203,32 @@ const noOrDieCompiledQuery = { }, } +const NO_TRY_CATCH_MESSAGE = + "Do not use `try`/`catch`. A thrown exception is invisible to the type system, so it escapes the typed error channel, and one `catch` block flattens every failure into a single branch. Use the Effect primitive: `Effect.try`/`Effect.tryPromise` for a throwing call, `Schema.fromJsonString` for JSON, `Schema.decodeUnknown{Effect,Option,Sync}` for decoding, `Effect.catch`/`catchTag`/`catchDefect` to handle a failure, and `Effect.ensuring`/`Effect.addFinalizer` for a `finally`." + +const noTryCatch = { + meta: { + type: "problem", + docs: { + description: "Disallow `try`/`catch`/`finally` in favour of Effect's typed error channel.", + }, + messages: { noTryCatch: NO_TRY_CATCH_MESSAGE }, + }, + create(context) { + return { + TryStatement(node) { + context.report({ node, messageId: "noTryCatch" }) + }, + } + }, +} + export default { meta: { name: "maple" }, rules: { "no-ordie-compiled-query": noOrDieCompiledQuery, "no-react-use-effect": noReactUseEffect, "no-record-string-any": noRecordStringAny, + "no-try-catch": noTryCatch, }, }