improvement(logs): drop casts and test-only exports from the failure log

Reads status and cause without casts, keeps the thrown missing-required-fields
error's name and stack unchanged, ignores an empty execution id when scoping a
logged mark, unexports wasFailureLogged and FailureKind (tests observe the
outer boundary through logFailureOnce instead), and shrinks the explicit-any
baseline for the Agent handler.

Author: waleedlatif1

apps/sim/executor/execution/block-executor.test.ts

@@ -1,11 +1,12 @@
+import { createLogger } from '@sim/logger'
 import { loggerMock } from '@sim/testing'
 import { maskClientMock, maskClientMockFns } from '@sim/testing/mocks/mask-client.mock'
 import { permissionCheckMock } from '@sim/testing/mocks/permission-check.mock'
 import { storageServiceMockFns } from '@sim/testing/mocks/storage-service.mock'
 import { uploadsMock } from '@sim/testing/mocks/uploads.mock'
 import { DrizzleQueryError } from 'drizzle-orm/errors'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
-import { classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log'
+import { classifyFailure, logFailureOnce } from '@/lib/core/errors/failure-log'
 import { clearLargeValueCacheForTests } from '@/lib/execution/payloads/cache'
 import { createLargeArrayManifest } from '@/lib/execution/payloads/large-array-manifest'
 import { isLargeValueRef } from '@/lib/execution/payloads/large-value-ref'
@@ -500,7 +501,7 @@ describe('BlockExecutor', () => {
     expect(JSON.stringify(logged)).not.toContain('owner-secret-id')
     /** Logged here at error, so the engine and the run surfaces must see it as already logged. */
     expect(classifyFailure(thrown)).toBe('internal')
-    expect(wasFailureLogged(thrown)).toBe(true)
+    expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', thrown)).toBeUndefined()
   })
 
   it('fires block completion callbacks for pausing blocks so clients receive pause output', async () => {

apps/sim/executor/execution/failure-trace.test.ts

@@ -1,6 +1,7 @@
+import { createLogger } from '@sim/logger'
 import { permissionCheckMock } from '@sim/testing/mocks/permission-check.mock'
 import { describe, expect, it, vi } from 'vitest'
-import { classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log'
+import { classifyFailure, logFailureOnce } from '@/lib/core/errors/failure-log'
 import { buildTraceSpans } from '@/lib/logs/execution/trace-spans/trace-spans'
 import { DAGExecutor } from '@/executor/execution/executor'
 import { hasExecutionResult } from '@/executor/utils/errors'
@@ -87,7 +88,7 @@ describe('failed run trace', () => {
      * The block executor logged it; the block wrap and the engine's rethrow must keep that
      * visible so execution-core and the trigger surfaces do not log it again.
      */
-    expect(wasFailureLogged(thrown)).toBe(true)
+    expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', thrown)).toBeUndefined()
     expect(classifyFailure(thrown)).toBe('user')
   })
 })

apps/sim/executor/handlers/agent/agent-handler.ts

@@ -1,6 +1,6 @@
 import { createLogger } from '@sim/logger'
 import { getErrorMessage, toError } from '@sim/utils/errors'
-import { isPlainRecord, omit } from '@sim/utils/object'
+import { isPlainRecord, isRecordLike, omit } from '@sim/utils/object'
 import { truncate } from '@sim/utils/string'
 import { markFailureKind } from '@/lib/core/errors/failure-log'
 import { normalizeStringRecord, normalizeWorkflowVariables } from '@/lib/core/utils/records'
@@ -328,7 +328,7 @@ function describeProviderTransportFailure(error: Error): string | null {
 }
 
 function isProviderKeyRejection(error: unknown): boolean {
-  const status = (error as { status?: unknown } | null)?.status
+  const status = isRecordLike(error) ? error.status : undefined
   return status === 401 || status === 402 || status === 403
 }
 

apps/sim/executor/handlers/workflow/workflow-handler.test.ts

@@ -1,3 +1,4 @@
+import { createLogger } from '@sim/logger'
 import { encryptionMockFns, environmentUtilsMockFns, resetEnvironmentUtilsMock } from '@sim/testing'
 import { createSessionPrincipal } from '@sim/testing/factories/principal.factory'
 import { authInternalMock, authInternalMockFns } from '@sim/testing/mocks/auth-internal.mock'
@@ -18,7 +19,7 @@ import {
 import { permissionsMock } from '@sim/testing/mocks/permissions.mock'
 import { usersQueriesMock, usersQueriesMockFns } from '@sim/testing/mocks/users-queries.mock'
 import { afterAll, beforeAll, beforeEach, describe, expect, it, type Mock, vi } from 'vitest'
-import { classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log'
+import { classifyFailure, logFailureOnce } from '@/lib/core/errors/failure-log'
 import { createTimeoutAbortController, getExecutionDeadlineAt } from '@/lib/core/execution-limits'
 import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { getBlock } from '@/blocks/registry'
@@ -382,7 +383,7 @@ describe('WorkflowBlockHandler', () => {
 
       expect(thrown).toBeInstanceOf(Error)
       /** Logged here, the block executor would skip its log carrying block, run, and stack. */
-      expect(wasFailureLogged(thrown)).toBe(false)
+      expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', thrown)).toBe('internal')
       expect(classifyFailure(thrown)).toBe('internal')
     })
 

apps/sim/executor/handlers/workflow/workflow-handler.ts

@@ -1575,7 +1575,7 @@ export class WorkflowBlockHandler implements BlockHandler {
         childWorkflowSnapshotId,
         childWorkflowInstanceId: instanceId,
       })
-      /** The child run logged its own failure; the warning above names it for this run. */
+      /** The warning above is this failure's log line. */
       markFailureLogged(childFailure)
       throw childFailure
     }

apps/sim/lib/core/errors/failure-log.test.ts

@@ -7,12 +7,18 @@ import {
   logFailureOnce,
   markFailureKind,
   markFailureLogged,
-  wasFailureLogged,
 } from '@/lib/core/errors/failure-log'
 import { RetryableSetupError } from '@/lib/core/errors/retryable-infrastructure'
 import { UserFailure } from '@/lib/core/errors/user-failure'
 import { HostedKeyRateLimitedError, HostedKeyUnavailableError } from '@/tools/errors'
 
+const logger = createLogger('FailureLogTest')
+
+/** What an outer boundary does with the failure: the kind it logs at, or undefined when it skips. */
+function outerBoundary(error: unknown, executionId?: string) {
+  return logFailureOnce(logger, 'probe', error, { executionId })
+}
+
 describe('classifyFailure', () => {
   it('keeps a database failure internal even beneath a user mark', () => {
     const queryError = new DrizzleQueryError('select 1', [], new Error('connection reset'))
@@ -52,8 +58,7 @@ describe('classifyFailure', () => {
     const first = new Error('first')
     const second = new Error('second', { cause: first })
     Object.assign(first, { cause: second })
-    expect(classifyFailure(first)).toBe('internal')
-    expect(wasFailureLogged(first)).toBe(false)
+    expect(outerBoundary(first)).toBe('internal')
   })
 })
 
@@ -62,37 +67,35 @@ describe('inheritFailureMarks', () => {
     const logged = new Error('child failed')
     markFailureLogged(logged)
     const boundary = inheritFailureMarks(new Error('Custom block execution failed'), logged)
-    expect(wasFailureLogged(boundary)).toBe(true)
+    expect(outerBoundary(boundary)).toBeUndefined()
     expect(classifyFailure(boundary)).toBe('internal')
 
     const authored = inheritFailureMarks(new Error('wrapped'), new UserFailure('missing input'))
-    expect(wasFailureLogged(authored)).toBe(false)
-    expect(classifyFailure(authored)).toBe('user')
+    expect(outerBoundary(authored)).toBe('user')
   })
 })
 
-describe('wasFailureLogged', () => {
+describe('logFailureOnce', () => {
   it('marks a frozen error, which a property write would throw on', () => {
     const frozen = Object.freeze(new Error('frozen'))
     markFailureLogged(frozen)
     markFailureKind(frozen, 'user')
-    expect(wasFailureLogged(frozen)).toBe(true)
+    expect(outerBoundary(frozen)).toBeUndefined()
     expect(classifyFailure(frozen)).toBe('user')
   })
 
   it('does not treat an unrelated error as logged', () => {
     markFailureLogged(new Error('logged elsewhere'))
-    expect(wasFailureLogged(new Error('fresh'))).toBe(false)
+    expect(outerBoundary(new Error('fresh'))).toBe('internal')
   })
 
   it('scopes a raw value logged at an execution boundary to that execution only', () => {
     const persistentFault = new Error('module failed to load')
-    logFailureOnce(createLogger('FailureLogTest'), 'Execution failed', persistentFault, {
-      executionId: 'exec-1',
-    })
+    expect(outerBoundary(persistentFault, 'exec-1')).toBe('internal')
 
-    expect(wasFailureLogged(persistentFault, 'exec-1')).toBe(true)
-    expect(wasFailureLogged(persistentFault, 'exec-2')).toBe(false)
-    expect(wasFailureLogged(persistentFault)).toBe(false)
+    expect(outerBoundary(persistentFault, 'exec-1')).toBeUndefined()
+    expect(outerBoundary(persistentFault, 'exec-2')).toBe('internal')
+    expect(outerBoundary(persistentFault)).toBe('internal')
+    expect(outerBoundary(persistentFault, '')).toBe('internal')
   })
 })

apps/sim/lib/core/errors/failure-log.ts

@@ -16,9 +16,9 @@ import { HttpError } from '@/lib/core/utils/http-error'
  *   in-process operations are attributed explicitly at their boundary and never land here.
  * - `internal`: Sim's own fault, or a cause nothing attributed. Always logged at error.
  */
-export type FailureKind = 'user' | 'third_party_client' | 'third_party_server' | 'internal'
+type FailureKind = 'user' | 'third_party_client' | 'third_party_server' | 'internal'
 
-/** Mirrors the cause depth `describeError` and `readStatusCode` follow. */
+/** Same bound `readStatusCode` puts on its cause walk. */
 const MAX_CAUSE_DEPTH = 8
 
 /**
@@ -45,7 +45,7 @@ function causeChain(error: unknown): object[] {
   let current = error
   while (isKeyable(current) && chain.length < MAX_CAUSE_DEPTH && !chain.includes(current)) {
     chain.push(current)
-    current = (current as { cause?: unknown }).cause
+    current = 'cause' in current ? current.cause : undefined
   }
   return chain
 }
@@ -75,7 +75,7 @@ export function classifyFailure(error: unknown): FailureKind {
       return link.statusCode >= 400 && link.statusCode < 500 ? 'user' : 'internal'
     }
 
-    const status = (link as { status?: unknown }).status
+    const status = 'status' in link ? link.status : undefined
     if (typeof status === 'number' && status >= 400 && status < 600) {
       return status >= 500 ? 'third_party_server' : 'third_party_client'
     }
@@ -119,11 +119,11 @@ export function adoptToolFailure<T>(error: T, result: { output?: unknown }): T {
  * Whether a boundary already logged this failure: a logged carrier anywhere in the cause chain,
  * or, given `executionId`, a raw value that boundary logged during this same execution.
  */
-export function wasFailureLogged(error: unknown, executionId?: string): boolean {
+function wasFailureLogged(error: unknown, executionId?: string): boolean {
   return causeChain(error).some(
     (link) =>
       loggedCarriers.has(link) ||
-      (executionId !== undefined && loggedInExecution.get(link) === executionId)
+      (Boolean(executionId) && loggedInExecution.get(link) === executionId)
   )
 }
 
@@ -159,10 +159,11 @@ export function logFailureOnce(
   const failureKind = classifyFailure(error)
   const fields = typeof metadata === 'function' ? metadata() : metadata
   logger[LOG_LEVEL_BY_KIND[failureKind]](message, {
-    ...(executionId !== undefined ? { executionId } : {}),
+    ...(executionId ? { executionId } : {}),
     ...fields,
     failureKind,
   })
-  if (executionId !== undefined && isKeyable(error)) loggedInExecution.set(error, executionId)
+  /** An empty id (a request that failed before minting one) scopes nothing. */
+  if (executionId && isKeyable(error)) loggedInExecution.set(error, executionId)
   return failureKind
 }

apps/sim/serializer/errors.ts

@@ -4,6 +4,5 @@ import { UserFailure } from '@/lib/core/errors/user-failure'
 export class MissingRequiredFieldsError extends UserFailure {
   constructor(blockName: string, missingFields: string[]) {
     super(`${blockName} is missing required fields: ${missingFields.join(', ')}`)
-    this.name = 'MissingRequiredFieldsError'
   }
 }

apps/sim/serializer/index.test.ts

@@ -369,8 +369,10 @@ describe('Serializer', () => {
       } catch (error) {
         refusal = error
       }
-      expect(refusal).toBeInstanceOf(Error)
-      expect((refusal as Error).message).toBe('Test Jina Block is missing required fields: API Key')
+      expect(refusal).toMatchObject({
+        name: 'Error',
+        message: 'Test Jina Block is missing required fields: API Key',
+      })
       /** The author's configuration, so execution logs it at info rather than paging at error. */
       expect(classifyFailure(refusal)).toBe('user')
     })

apps/sim/tools/index.test.ts

@@ -1,3 +1,4 @@
+import { createLogger } from '@sim/logger'
 import { createSessionPrincipal } from '@sim/testing/factories/principal.factory'
 import { jsonResponse } from '@sim/testing/helpers/http'
 import { apiKeyByokMock, apiKeyByokMockFns } from '@sim/testing/mocks/api-key-byok.mock'
@@ -420,7 +421,7 @@ vi.mock('@/tools/utils.server', async (importOriginal) => {
 })
 
 import type { QueryClient } from '@tanstack/react-query'
-import { adoptToolFailure, classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log'
+import { adoptToolFailure, classifyFailure, logFailureOnce } from '@/lib/core/errors/failure-log'
 import * as getQueryClientModule from '@/app/_shell/providers/get-query-client'
 import { ApiBlockHandler } from '@/executor/handlers/api/api-handler'
 import { buildBlockExecutionError } from '@/executor/utils/errors'
@@ -5923,7 +5924,7 @@ describe('Centralized Error Handling', () => {
       async (status, kind) => {
         const blockError = await failApiBlock(jsonResponse({ error: 'rejected' }, status))
         expect(classifyFailure(blockError)).toBe(kind)
-        expect(wasFailureLogged(blockError)).toBe(true)
+        expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', blockError)).toBeUndefined()
       }
     )
 
@@ -5942,7 +5943,7 @@ describe('Centralized Error Handling', () => {
       try {
         const blockError = await failApiBlock(jsonResponse({ ok: false }))
         expect(classifyFailure(blockError)).toBe(kind)
-        expect(wasFailureLogged(blockError)).toBe(true)
+        expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', blockError)).toBeUndefined()
       } finally {
         tools.http_request.transformResponse = originalTransform
       }