mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
* fix(tables): retry transient DB/Redis failures in cell execution and surface error causes Workflow-group-cell runs intermittently failed on trivial DB reads/writes under heavy fan-out, stranding cells in `running`. Investigation showed the PlanetScale and ElastiCache backends were healthy at the time — the failures are transient connection-level faults that the cell (maxAttempts: 1) had no tolerance for, and the real cause was never logged (Drizzle wraps it as "Failed query: ..." and the driver cause lives in error.cause). Resilience: - Add retryTransient (lib/table/retry-transient.ts): retries only transient infra errors (reuses isRetryableInfrastructureError; adds an ioredis command-timeout match) with jittered backoff, then rethrows. Fail-fast for everything else. - Wrap the cell's getTableById/getRowById reads, the terminal write (cell-write updateRow — idempotent via the executionId guard), and the Redis cascade-lock acquire. Diagnostics: - Add describeError (lib/core/errors/retryable-infrastructure.ts): walks the .cause chain and always returns the underlying driver cause (code/errno/ syscall + causeChain), including for unclassified errors like AbortError. - Log `cause` + a `retryable` flag (and aborted/timedOut in the cell's main catch) across the cell + finalization error paths, mirroring the existing schedule-execution pattern. Logging-only; no behavior change. This lets the next recurrence reveal the real cause and whether the retry applies. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(tables): address review feedback on cell retry resilience - retryTransient: re-check the abort signal after the backoff sleep so a cancellation during sleep stops the next attempt (don't run/return work for an already-cancelled task). - isRetryableRedisError: walk the .cause chain (mirroring the infra classifier) so wrapped Redis timeouts are recognized; drop "Connection is in subscriber mode" — that's a connection-state programming error, not a transient drop, and would just fail identically every retry. - cascade-lock: stop wrapping acquireLock in retryTransient. acquireLock is a non-idempotent SET NX, so retrying after a timed-out-but-applied first SET returns false (key already ours) and yields a false `contended` that skips the cascade. A transient Redis blip here just fails the run before pickup (no stranded cell); the dispatcher re-drives it. - Tests: cause-chain Redis match, subscriber-mode exclusion, abort-during-sleep. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(tables): drop out-of-scope abort/timeout fields from cell catch The main catch logged `aborted`/`timedOut` from `abortSignal`/`timeoutController`, but those are declared inside the outer try block (the inner try around executeWorkflow is try/finally, so this catch belongs to the outer try) and are not in scope in the catch — `next build`'s type-check failed with "Cannot find name 'abortSignal'". Local incremental `tsc --noEmit` had skipped the file and falsely passed; the Cursor/Greptile reviewers flagged this correctly. Removed the two fields. Abort/timeout is still surfaced via `cause: describeError(err)` (an aborted run shows `name: 'AbortError'` / the timeout message), so no diagnostic signal is lost. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(tables): drop in-process retry, keep cause diagnostics only In-process retry is the wrong layer for this path: the cell task is maxAttempts:1 by design, retrying on a possibly-degraded worker may not help, and it masks the very transient-failure signal we're trying to capture before we understand the root cause. Removed retryTransient entirely (file + all wrapping in cell-write, the cascade reads, and the lock acquire) and kept only the diagnostic logging. - Deleted lib/table/retry-transient.ts (+ test); cell-write and the cascade reads call getTableById/getRowById/updateRow directly again, fail-fast. - Kept describeError + `cause`/`retryable` fields across the cell + finalization catch blocks; the cell-path `retryable` flag now sources from isRetryableInfrastructureError (the canonical classifier) for consistency. Diagnostics-first: surface the real driver cause on the next recurrence, then decide the actual fix (e.g. task-level maxAttempts, or addressing the worker- side cause) from evidence rather than a speculative in-process retry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(schedules): log error cause on scheduled-execution failure paths The scheduled-job failure paths logged the raw error (.message/stack only) — its `.cause` (the real driver error behind a Drizzle "Failed query: ..." wrapper) was never recorded, and the classified-only `describeRetryableInfrastructureError` returns undefined for unrecognized errors. A real failed run (same incident window as the cell failures) failed in `applyScheduleUpdate` with exactly this unrecorded cause. Added `cause: describeError(error)` (always-on, walks the cause chain) to the applyScheduleUpdate catch, the early-failure catch, and the unhandled-error catch — passed as a second arg so the existing message+stack still emit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(errors): move describeError to @sim/utils/errors `describeError` is a general-purpose error/cause-chain helper — it didn't belong in `lib/core/errors/retryable-infrastructure.ts` (that module is specifically about classifying retryable infra errors, and the name read wrong for a generic diagnostic). Moved it to `@sim/utils/errors` alongside `toError`/ `getErrorMessage`/`getPostgresErrorCode`, with its own cycle-safe cause walk. - Added describeError + DescribedError + tests to packages/utils/src/errors.ts. - Reverted the describeError addition from retryable-infrastructure.ts (it keeps only isRetryableInfrastructureError / describeRetryableInfrastructureError, which are accurately named and still used by the schedule retry path). - Re-pointed all consumers (cell, logging-session, pause-persistence, schedule) to import describeError from @sim/utils/errors. The `retryable` classification flag still sources from isRetryableInfrastructureError where used. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
123 lines
4.1 KiB
TypeScript
123 lines
4.1 KiB
TypeScript
/**
|
|
* Normalizes an unknown caught value into an Error instance.
|
|
* Replaces the common `e instanceof Error ? e : new Error(String(e))` pattern in catch clauses.
|
|
*/
|
|
export function toError(value: unknown): Error {
|
|
if (value instanceof Error) return value
|
|
if (typeof value === 'string') return new Error(value)
|
|
return new Error(String(value))
|
|
}
|
|
|
|
/**
|
|
* Extracts a string message from an unknown caught value.
|
|
* Use instead of `e instanceof Error ? e.message : 'fallback'` in catch clauses.
|
|
*
|
|
* - Error instance → `error.message`
|
|
* - Non-empty string → the string itself (handles `throw 'msg'` patterns)
|
|
* - Otherwise → `fallback` if provided, or `String(value)`
|
|
*/
|
|
export function getErrorMessage(value: unknown, fallback?: string): string {
|
|
if (value instanceof Error) return value.message
|
|
if (typeof value === 'string' && value.length > 0) return value
|
|
return fallback ?? String(value)
|
|
}
|
|
|
|
/**
|
|
* Returns PostgreSQL error code (e.g. `23505` for unique_violation) when present on a thrown value.
|
|
* Normalizes common Drizzle / `postgres` driver shapes and walks `cause` chains.
|
|
*/
|
|
export function getPostgresErrorCode(error: unknown): string | undefined {
|
|
return readPgErrorField(error, 'code')
|
|
}
|
|
|
|
/**
|
|
* Returns the name of the PostgreSQL constraint that triggered the error (e.g. the unique index
|
|
* name on a `23505`), when present on a thrown value. Mirrors the field populated by the
|
|
* `postgres` / `pg` drivers, walking `cause` chains the same way as `getPostgresErrorCode`.
|
|
*/
|
|
export function getPostgresConstraintName(error: unknown): string | undefined {
|
|
return readPgErrorField(error, 'constraint_name') ?? readPgErrorField(error, 'constraint')
|
|
}
|
|
|
|
export interface DescribedError {
|
|
name: string
|
|
message: string
|
|
code?: string
|
|
errno?: string
|
|
syscall?: string
|
|
/** `"Name: message"` per link in the `.cause` chain, outermost first. Present only when the chain has more than one link. */
|
|
causeChain?: string[]
|
|
}
|
|
|
|
/**
|
|
* Always-on diagnostic view of an error and its `.cause` chain.
|
|
*
|
|
* Reports the fields of the DEEPEST `.cause` link, because a wrapped driver
|
|
* error (e.g. Drizzle's `"Failed query: ..."` wrapping an `ECONNRESET`) carries
|
|
* the real reason there, not on the outer wrapper. Always returns a populated
|
|
* object — including for non-`Error` throws and unclassified errors like
|
|
* `AbortError`. Cycle-safe and depth-bounded.
|
|
*
|
|
* Loggers do not serialize the non-enumerable `Error.prototype.cause`, so pass
|
|
* the result as an explicit structured field rather than the raw error.
|
|
*/
|
|
export function describeError(error: unknown): DescribedError {
|
|
const chain: Error[] = []
|
|
const seen = new Set<unknown>()
|
|
let current: unknown = error
|
|
while (current instanceof Error && !seen.has(current) && chain.length < 10) {
|
|
seen.add(current)
|
|
chain.push(current)
|
|
current = current.cause
|
|
}
|
|
|
|
if (chain.length === 0) {
|
|
const normalized = toError(error)
|
|
return { name: normalized.name, message: normalized.message }
|
|
}
|
|
|
|
const deepest = chain[chain.length - 1] as Error & Record<string, unknown>
|
|
const asString = (value: unknown): string | undefined =>
|
|
typeof value === 'string' ? value : undefined
|
|
const code = asString(deepest.code)
|
|
const errno = asString(deepest.errno)
|
|
const syscall = asString(deepest.syscall)
|
|
|
|
return {
|
|
name: deepest.name,
|
|
message: deepest.message,
|
|
...(code ? { code } : {}),
|
|
...(errno ? { errno } : {}),
|
|
...(syscall ? { syscall } : {}),
|
|
...(chain.length > 1 ? { causeChain: chain.map((e) => `${e.name}: ${e.message}`) } : {}),
|
|
}
|
|
}
|
|
|
|
function readPgErrorField(error: unknown, field: string): string | undefined {
|
|
const seen = new Set<unknown>()
|
|
let current: unknown = error
|
|
|
|
while (current !== undefined && current !== null) {
|
|
if (seen.has(current)) {
|
|
break
|
|
}
|
|
seen.add(current)
|
|
|
|
if (typeof current === 'object') {
|
|
const value = (current as Record<string, unknown>)[field]
|
|
if (typeof value === 'string') {
|
|
return value
|
|
}
|
|
}
|
|
|
|
if (current instanceof Error && current.cause !== undefined) {
|
|
current = current.cause
|
|
continue
|
|
}
|
|
|
|
break
|
|
}
|
|
|
|
return undefined
|
|
}
|