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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions apps/sim/lib/api/server/routes/internal-json-route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ import {
InvalidInternalDelegationBindingError,
} from '@/lib/auth/internal-delegation'
import type { ApplicationOperation, OperationUseCase } from '@/lib/core/application'
import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types'
import {
asOrchestrationError,
messageForOrchestrationError,
statusForOrchestrationError,
} from '@/lib/core/orchestration/types'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'

export class InternalUnauthenticatedError extends Error {
Expand Down Expand Up @@ -142,7 +146,10 @@ export const internalOrchestrationErrorPolicy: InternalErrorPolicy = {
const classified = asOrchestrationError(error)
if (!classified) return null
return internalErrorResponse(statusForOrchestrationError(classified.code), {
error: classified.message,
error: messageForOrchestrationError(
{ error: classified.message, errorCode: classified.code },
'Internal server error'
),
})
},
unhandled() {
Expand Down
69 changes: 68 additions & 1 deletion apps/sim/lib/core/orchestration/types.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,15 @@
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import { statusForOrchestrationError } from '@/lib/core/orchestration/types'
import {
messageForOrchestrationError,
OrchestrationError,
statusForOrchestrationError,
throwOrchestrationFailure,
} from '@/lib/core/orchestration/types'

const RAW_DRIVER_MESSAGE =
'insert into "workflow" ("id") values ($1) - duplicate key value violates unique constraint "workflow_pkey"'

describe('statusForOrchestrationError', () => {
it.each([
Expand All @@ -15,3 +23,62 @@ describe('statusForOrchestrationError', () => {
expect(statusForOrchestrationError(code)).toBe(expected)
})
})

describe('messageForOrchestrationError', () => {
it('withholds the message of an explicitly internal failure', () => {
expect(
messageForOrchestrationError(
{ error: RAW_DRIVER_MESSAGE, errorCode: 'internal' },
'Failed to create workflow'
)
).toBe('Failed to create workflow')
})

it('withholds the message of a failure carrying no code', () => {
expect(
messageForOrchestrationError({ error: RAW_DRIVER_MESSAGE }, 'Failed to create workflow')
).toBe('Failed to create workflow')
})

it('returns a classified failure message to the caller', () => {
expect(
messageForOrchestrationError(
{ error: 'Workflow name is already taken', errorCode: 'conflict' },
'Failed to create workflow'
)
).toBe('Workflow name is already taken')
})

it('falls back when a classified failure carries no message', () => {
expect(
messageForOrchestrationError({ errorCode: 'conflict' }, 'Failed to create workflow')
).toBe('Failed to create workflow')
})
})

describe('throwOrchestrationFailure', () => {
it('classifies an uncoded failure as internal without rendering its message', () => {
try {
throwOrchestrationFailure({ error: RAW_DRIVER_MESSAGE }, 'Failed to update workflow')
expect.unreachable('expected throwOrchestrationFailure to throw')
} catch (error) {
expect(error).toBeInstanceOf(OrchestrationError)
expect((error as OrchestrationError).code).toBe('internal')
expect((error as OrchestrationError).message).toBe('Failed to update workflow')
}
})

it('preserves the code and message of a classified failure', () => {
try {
throwOrchestrationFailure(
{ error: 'No such workflow', errorCode: 'not_found' },
'Failed to delete workflow'
)
expect.unreachable('expected throwOrchestrationFailure to throw')
} catch (error) {
expect(error).toBeInstanceOf(OrchestrationError)
expect((error as OrchestrationError).code).toBe('not_found')
expect((error as OrchestrationError).message).toBe('No such workflow')
}
})
})
19 changes: 19 additions & 0 deletions apps/sim/lib/core/orchestration/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,25 @@ export class OrchestrationError extends Error {
}
}

/**
* Rethrows a failed orchestration result as its classified {@link OrchestrationError}.
*
* Pairs the code with the message {@link messageForOrchestrationError} permits for
* it, so the two can never disagree. Hand-rolling that pair is what let raw driver
* text reach clients: a site that defaulted the code with `?? 'internal'` but then
* compared the *raw* `errorCode` against `'internal'` classified an uncoded failure
* as internal while still rendering its own message.
*/
export function throwOrchestrationFailure(
result: { error?: string; errorCode?: OrchestrationErrorCode },
fallback: string
): never {
throw new OrchestrationError(
result.errorCode ?? 'internal',
messageForOrchestrationError(result, fallback)
)
}

/**
* The {@link OrchestrationError} in `error`'s cause chain, or `null` when the
* failure is not a classified one.
Expand Down
11 changes: 6 additions & 5 deletions apps/sim/lib/knowledge/application/folders.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { AuditAction, AuditResourceType } from '@sim/audit'
import type { folder } from '@sim/db/schema'
import { OrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types'
import {
OrchestrationError,
type OrchestrationErrorCode,
throwOrchestrationFailure,
} from '@/lib/core/orchestration/types'
import {
createFolderAtPath,
deleteFolderByPath,
Expand Down Expand Up @@ -49,10 +53,7 @@ export interface DeleteKnowledgeFolderInput {
}

function throwFolderFailure(result: { error?: string; errorCode?: OrchestrationErrorCode }): never {
throw new OrchestrationError(
result.errorCode ?? 'internal',
result.error ?? 'Folder operation failed'
)
throwOrchestrationFailure(result, 'Folder operation failed')
}

export const listKnowledgeFolders = defineAuthorizedKnowledgeUseCase({
Expand Down
39 changes: 39 additions & 0 deletions apps/sim/lib/workflows/application/transition-result.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import { OrchestrationError } from '@/lib/core/orchestration/types'
import { requireWorkflowTransition } from '@/lib/workflows/application/transition-result'

describe('requireWorkflowTransition', () => {
it('returns without throwing for a successful transition', () => {
expect(() => requireWorkflowTransition({ success: true }, 'Failed')).not.toThrow()
})

it('withholds the raw message a failed lifecycle transition carries', () => {
expect(() =>
requireWorkflowTransition(
{
success: false,
error: 'duplicate key value violates unique constraint "workflow_pkey"',
errorCode: 'internal',
},
'Failed to create workflow'
)
).toThrow('Failed to create workflow')
})

it('preserves a classified failure so the route maps the right status', () => {
try {
requireWorkflowTransition(
{ success: false, error: 'No such workflow', errorCode: 'not_found' },
'Failed to delete workflow'
)
expect.unreachable('expected requireWorkflowTransition to throw')
} catch (error) {
expect(error).toBeInstanceOf(OrchestrationError)
expect((error as OrchestrationError).code).toBe('not_found')
expect((error as OrchestrationError).message).toBe('No such workflow')
}
})
})
7 changes: 5 additions & 2 deletions apps/sim/lib/workflows/application/transition-result.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { OrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types'
import {
type OrchestrationErrorCode,
throwOrchestrationFailure,
} from '@/lib/core/orchestration/types'

export function requireWorkflowTransition<
T extends { success: boolean; error?: string; errorCode?: OrchestrationErrorCode },
>(result: T, fallbackMessage: string): asserts result is T & { success: true } {
if (result.success) return
throw new OrchestrationError(result.errorCode ?? 'internal', result.error ?? fallbackMessage)
throwOrchestrationFailure(result, fallbackMessage)
}
8 changes: 2 additions & 6 deletions apps/sim/lib/workflows/application/workflow-folders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { AuditAction, AuditResourceType } from '@sim/audit'
import { resolvePrincipalAttribution } from '@sim/auth/principal'
import type { folder } from '@sim/db/schema'
import type { OrchestrationErrorCode } from '@/lib/core/orchestration/types'
import { OrchestrationError } from '@/lib/core/orchestration/types'
import { OrchestrationError, throwOrchestrationFailure } from '@/lib/core/orchestration/types'
import { MAX_FOLDERS_PER_WORKSPACE } from '@/lib/folders/constants'
import { withFolderTreeLock } from '@/lib/folders/locks'
import {
Expand Down Expand Up @@ -73,11 +73,7 @@ function throwFolderMutationFailure(result: {
error?: string
errorCode?: OrchestrationErrorCode
}): never {
const code = result.errorCode ?? 'internal'
throw new OrchestrationError(
code,
code === 'internal' ? 'Internal server error' : (result.error ?? 'Folder mutation failed')
)
throwOrchestrationFailure(result, 'Internal server error')
}

export async function resolveWorkflowFolderPath(
Expand Down
8 changes: 2 additions & 6 deletions apps/sim/lib/workflows/application/workflow-vfs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
asOrchestrationError,
OrchestrationError,
type OrchestrationErrorCode,
throwOrchestrationFailure,
} from '@/lib/core/orchestration/types'
import { generateRequestId } from '@/lib/core/utils/request'
import {
Expand Down Expand Up @@ -246,12 +247,7 @@ function resolveWorkflowSources(
}

function throwFolderFailure(result: { error?: string; errorCode?: OrchestrationErrorCode }): never {
throw new OrchestrationError(
result.errorCode ?? 'internal',
result.errorCode === 'internal'
? 'Workflow folder mutation failed'
: (result.error ?? 'Folder mutation failed')
)
throwOrchestrationFailure(result, 'Workflow folder mutation failed')
}

async function reloadFolderIndex(state: WorkflowVfsIndexState, workspaceId: string): Promise<void> {
Expand Down
Loading