fix(vm): categorize user or server side errors

[TheodoreSpeaks]

Summary

Brief description of what this PR does and why.

Fixes #(issue)

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation
• Other: ___________

Testing

How has this been tested? What should reviewers focus on?

Checklist

• Code follows project style guidelines
• Self-reviewed my changes
• Tests added/updated and passing
• No new warnings introduced
• I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Screenshots/Videos

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Apr 24, 2026 0:30am

[TheodoreSpeaks]

@BugBot review

[cursor]

PR Summary

Medium Risk
Changes how isolated-vm and preview APIs classify and surface sandbox errors (422 vs 500), which can affect client behavior and monitoring/alerting if misclassified.

Overview
Improves sandbox error categorization by introducing IsolatedVMError.isSystemError and using it in runSandboxTask to throw a dedicated SandboxUserCodeError for user-caused failures while treating infrastructure/pool/task failures as system errors.

Document preview endpoints now catch SandboxUserCodeError and return 422 with { error, errorName } (and warn-level logging) instead of always returning 500; tests for PDF/PPTX/DOCX previews were updated to cover the new 422 behavior.

^{Reviewed by Cursor Bugbot for commit e3e3ea9. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit e3e3ea9. Configure here.}

[Siddhartha-singh01]

This is a solid improvement to error handling. Separating user-generated sandbox errors from server errors makes the API behavior much clearer and more predictable.

What’s working well:

The distinction between user errors (422) and system errors (500) is a great call and aligns well with HTTP semantics. Logging now includes useful context like workspaceId and the error name, which will make debugging much easier. The added test coverage also does a good job validating the new behavior.

A few things worth tightening up:

The use of instanceof SandboxUserCodeError could become fragile, especially across module boundaries. It might be safer to introduce a more stable way to identify these errors, like a discriminator property.

The response shape is slightly inconsistent. In one case you return { error, errorName }, and in the other just { error }. It would be cleaner to standardize this so clients can rely on a consistent structure.

In the tests, redefining SandboxUserCodeError locally could cause subtle issues if it doesn’t perfectly match the real implementation. Importing or mocking the actual class would make this more reliable.

There’s also a bit of duplication in the logging logic between the two branches. Pulling that into a small helper would make the code easier to maintain.

One small nit: the log message “preview user code failed” could be a bit clearer something like “user preview execution failed” reads more naturally.

Overall, this is a meaningful improvement and looks good to merge with a few minor tweaks.

[greptile-apps]

Greptile Summary

This PR introduces a structured error-classification layer between the isolated-VM execution pool and the document-preview API routes. Infrastructure failures (worker crash, dispatch failure, pool saturation, queue timeout, missing broker) are tagged isSystemError: true inside isolated-vm.ts and surface as a plain Error → HTTP 500, while all user-code failures (syntax errors, runtime exceptions, etc.) now throw SandboxUserCodeError → HTTP 422. Logging is also split: system errors use logger.error; user errors use logger.warn, keeping server-health dashboards clean of user-generated noise.

Confidence Score: 5/5

Safe to merge — all changed paths are well-tested and the error-classification logic is sound.

No P0/P1 findings. The isSystemError flag is correctly applied to every infrastructure error site in the diff, user-code errors correctly default to the user-caused branch, the instanceof check in the route handler is properly guarded, and the test stubs correctly export the hoisted class through the mock factory so instanceof resolves against the same reference at test time.

No files require special attention.

Important Files Changed

Filename	Overview
apps/sim/lib/execution/isolated-vm.ts	Adds isSystemError?: boolean to IsolatedVMError and tags all host-infrastructure failure sites (worker crash, dispatch failure, queue capacity, queue timeout, missing broker) with isSystemError: true; user-code errors remain untagged, correctly defaulting to user-caused.
apps/sim/lib/execution/sandbox/run-task.ts	Introduces SandboxUserCodeError class and splits error handling: system errors use logger.error and rethrow as plain Error, while user-code errors use logger.warn and rethrow as SandboxUserCodeError for 4xx translation upstream.
apps/sim/app/api/workspaces/[id]/_preview/create-preview-route.ts	Catches SandboxUserCodeError before the generic catch-all, logs at WARN level, and returns HTTP 422 with both error and errorName fields; server-side failures continue to surface as 500.
apps/sim/app/api/workspaces/[id]/docx/preview/route.test.ts	Adds 422 test case; correctly defines a hoisted SandboxUserCodeError stub and exports it via the vi.mock factory so instanceof checks in the route handler resolve against the same class reference.
apps/sim/app/api/workspaces/[id]/pdf/preview/route.test.ts	Mirrors the DOCX test pattern for the PDF route, adding the identical 422 test and hoisted class stub.
apps/sim/app/api/workspaces/[id]/pptx/preview/route.test.ts	Mirrors the DOCX test pattern for the PPTX route, adding the identical 422 test and hoisted class stub.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[runSandboxTask] --> B{result.error?}
    B -- No --> C[Return Buffer]
    B -- Yes --> D{isSystemError === true?}
    D -- Yes\nworker crash / dispatch failure\npool saturation / queue timeout\nmissing broker --> E[logger.error\nthrow plain Error]
    D -- No\nuser SyntaxError / ReferenceError\nuser throw / timeout --> F[logger.warn\nthrow SandboxUserCodeError]
    E --> G[create-preview-route catch]
    F --> G
    G --> H{instanceof SandboxUserCodeError?}
    H -- Yes --> I[HTTP 422\nerror + errorName]
    H -- No --> J[HTTP 500\nerror]

Loading

_{Reviews (1): Last reviewed commit: "fix(vm): categorize user or server side ..." | Re-trigger Greptile}