Repository navigation
Conversation
🦋 Changeset detectedLatest commit: 2bcd649 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…ated functions for clarity.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0f8defa. Configure here.
| this.natsBridgePort = options.natsBridgePort ?? DEFAULT_SANDBOX_NATS_WS_PORT; | ||
| this.previewUrlExpirySeconds = options.previewUrlExpirySeconds ?? DEFAULT_PREVIEW_URL_EXPIRY_SECONDS; | ||
| this.onCriticalAlert = options.onCriticalAlert; | ||
| this.onError = options.onError; |
There was a problem hiding this comment.
There are many places in this file has catch (error) {
Why should they not be piped to onError?
If they should not be piped then onError should be optional arg of getOrCreateSandbox?
| }); | ||
| this.onCriticalAlert?.(alertError); | ||
| this.onError?.(createError); | ||
| throw error; |
There was a problem hiding this comment.
why do we not throw createError?
Pass onError through getOrCreateSandbox so only fresh creates notify the host, and throw the normalized createError. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@thesujai lets either merge this or close this PR |
This branch has not been deployed
3 participants
Summary
Closes #
Changes
How was this tested?
Checklist
pnpm build,pnpm test,pnpm typecheck,pnpm lint:ci, andpnpm format:checkpass locallypackages/trueforge-sdk,python/trueforge_sdk,.github/fern/openapi/openapi.json,docs/openapi.json) — fork PRs omit SDK regen; maintainers regenerate after merge.env.exampleupdated if configuration or behavior changedNote
Low Risk
Alerting behavior is refactored but P1 Sentry reporting for non-auth create failures is preserved; auth errors are still excluded from P1 at the host layer.
Overview
Replaces Daytona provider
onCriticalAlertwith a host-ownedonErrorhook that runs only when a new sandbox create fails (viagetOrCreateSandbox→createNewSandbox). The provider no longer decides which failures are P1-worthy; it logs and invokes the hook, then rethrows a normalizedError.trueforgewiresonErrortocaptureCriticalExceptionwith the same P1 tags as before, but skips auth/permission errors in the host (isDaytonaAuthError/isDaytonaPermissionError) instead of inside the provider. Restore/exec/file paths callgetOrCreateSandboxwithoutonError, so they do not trigger that alerting path.Includes a patch changeset for
@truefoundry/trueforge-coreand@truefoundry/trueforge.Reviewed by Cursor Bugbot for commit 2bcd649. Bugbot is set up for automated code reviews on this repo. Configure here.