fix(vm): categorize user or server side errors#4283
fix(vm): categorize user or server side errors#4283TheodoreSpeaks wants to merge 1 commit intostagingfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@BugBot review |
PR SummaryMedium Risk Overview Document preview endpoints now catch Reviewed by Cursor Bugbot for commit e3e3ea9. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
✅ 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
left a comment
There was a problem hiding this comment.
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.
Summary
Brief description of what this PR does and why.
Fixes #(issue)
Type of Change
Testing
How has this been tested? What should reviewers focus on?
Checklist
Screenshots/Videos