-
Notifications
You must be signed in to change notification settings - Fork 176
Sandbox error handling #137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
jasonkneen
commented
Oct 21, 2025
- Add pre-flight validation of Vercel API credentials and timeout values
- Extract and log detailed error messages from Vercel API responses
- Add team/project ID logging (masked for security)
- Better error handling for 500 errors with Vercel status page reference
- More descriptive error messages for invalid timeout values
- Cap MAX_SANDBOX_DURATION to 40 minutes (Vercel API max is 45 minutes)
- Add Math.min() cap in creation.ts to prevent timeout > 2700000ms
- Add 60-second timeout for Sandbox.create() API call to prevent indefinite hanging
- Add diagnostic logging for sandbox configuration and creation initiation
- Improve error messaging for API credential issues
- Add pre-flight validation of Vercel API credentials and timeout values - Extract and log detailed error messages from Vercel API responses - Add team/project ID logging (masked for security) - Better error handling for 500 errors with Vercel status page reference - More descriptive error messages for invalid timeout values - Cap MAX_SANDBOX_DURATION to 40 minutes (Vercel API max is 45 minutes) - Add Math.min() cap in creation.ts to prevent timeout > 2700000ms - Add 60-second timeout for Sandbox.create() API call to prevent indefinite hanging - Add diagnostic logging for sandbox configuration and creation initiation - Improve error messaging for API credential issues
|
@jasonkneen is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves error handling and validation for Vercel sandbox creation by adding pre-flight checks, better error messages, and timeout management to prevent API failures and indefinite hangs.
Key changes:
- Adds pre-flight validation for Vercel API credentials (team ID, project ID, token) with descriptive error messages
- Implements a 60-second timeout for sandbox creation to prevent indefinite hanging
- Extracts and logs detailed error information from Vercel API responses, including special handling for 500 errors
- Caps sandbox timeout at 40 minutes (with Math.min enforcement) to stay within Vercel's 45-minute API limit
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lib/sandbox/creation.ts | Adds credential validation, 60s creation timeout, detailed error logging, and timeout capping logic |
| lib/constants.ts | Updates MAX_SANDBOX_DURATION default from 300 to 40 minutes with safety cap |
| .claude/settings.local.json | Adds local development permissions configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| export const MAX_SANDBOX_DURATION = parseInt(process.env.MAX_SANDBOX_DURATION || '300', 10) | ||
| // Sandbox configuration (in minutes) - Vercel Sandbox API has max 2700000ms (45 minutes) | ||
| // We cap at 40 minutes to have a safety buffer | ||
| export const MAX_SANDBOX_DURATION = Math.min(parseInt(process.env.MAX_SANDBOX_DURATION || '40', 10), 40) |
Copilot
AI
Oct 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Math.min() call is redundant and will always return 40. If MAX_SANDBOX_DURATION env var is set to a value less than 40, it will be ignored. This should be Math.min(parseInt(process.env.MAX_SANDBOX_DURATION || '40', 10), 40) which evaluates the parseInt first, then applies the cap.
| "Bash(rm:*)", | ||
| "Bash(npm run dev:*)", | ||
| "Bash(cat:*)", | ||
| "Bash(PGPASSWORD=\"npg_2zcXUvGdrM1A\" /opt/homebrew/opt/libpq/bin/psql -h \"ep-bold-sky-agftkbbb-pooler.c-2.eu-central-1.aws.neon.tech\" -U \"neondb_owner\" -d \"neondb\" -c \"SELECT 1\")", |
Copilot
AI
Oct 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Database password is exposed in plaintext in the permissions configuration. This credential should be removed and stored in environment variables or a secure credential store instead.
| "Bash(PGPASSWORD=\"npg_2zcXUvGdrM1A\" /opt/homebrew/opt/libpq/bin/psql -h \"ep-bold-sky-agftkbbb-pooler.c-2.eu-central-1.aws.neon.tech\" -U \"neondb_owner\" -d \"neondb\" -c \"SELECT 1\")", | |
| "Bash(PGPASSWORD=\"$DB_PASSWORD\" /opt/homebrew/opt/libpq/bin/psql -h \"ep-bold-sky-agftkbbb-pooler.c-2.eu-central-1.aws.neon.tech\" -U \"neondb_owner\" -d \"neondb\" -c \"SELECT 1\")", |
| // NOTE: Vercel Sandbox API has a maximum timeout of 2700000ms (45 minutes) | ||
| const configTimeoutMinutes = config.timeout ? parseInt(config.timeout.replace(/\D/g, '')) : 40 | ||
| const timeoutMs = Math.min(configTimeoutMinutes * 60 * 1000, 2700000) // Cap at 45 minutes |
Copilot
AI
Oct 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic number 2700000 appears multiple times in the code. Consider extracting it as a named constant like VERCEL_MAX_TIMEOUT_MS = 2700000 to improve maintainability and ensure consistency.
| if (timeoutMs <= 0 || timeoutMs > 2700000) { | ||
| throw new Error(`Invalid timeout value: ${timeoutMs}ms. Must be between 1 and 2700000ms (45 minutes).`) |
Copilot
AI
Oct 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation condition timeoutMs <= 0 is unreachable because the default is 40 minutes and Math.min ensures it never exceeds 2700000ms, meaning timeoutMs will always be positive. Consider removing this check or validating configTimeoutMinutes before the calculation.
| // Use the specified timeout (maxDuration) for sandbox lifetime | ||
| // keepAlive only controls whether we shutdown after task completion | ||
| const timeoutMs = config.timeout ? parseInt(config.timeout.replace(/\D/g, '')) * 60 * 1000 : 60 * 60 * 1000 // Default 1 hour | ||
| // NOTE: Vercel Sandbox API has a maximum timeout of 2700000ms (45 minutes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sandbox now has a max timeout of 5h! https://vercel.com/changelog/vercel-sandbox-maximum-duration-extended-to-5-hours
| @@ -0,0 +1,20 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this file
|
@jasonkneen Thank you so much! Left a couple of small comments. Everything else looks great 🙏 |