Conversation
🦋 Changeset detectedLatest commit: 7dd42b6 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 |
![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 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7dd42b6. Configure here.
| dsn, | ||
| includeLocalVariables: false, | ||
| integrations: [], | ||
| }); |
There was a problem hiding this comment.
Default Sentry integrations stay enabled
High Severity
integrations: [] does not turn off Sentry's default integrations. In @sentry/node v10 those still load unless defaultIntegrations is false, so enabling Sentry also installs process-wide onUncaughtException, onUnhandledRejection, and HTTP/fetch instrumentation. That reports more than the intended P1 captures and can hook http/undici used by the server.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7dd42b6. Configure here.
| ({ sandboxInfo, sandboxCreated } = await this.ensureReadySandbox()); | ||
| } catch (e) { | ||
| this.logger.error('Sandbox initialization failed', extractErrorLogFields(e)); | ||
| this.onInitFailure?.(e); |
There was a problem hiding this comment.
Sandbox init misses some failures
Medium Severity
onInitFailure runs only in handleExec's first ensureReadySandbox catch. Init failures from uploadFile / uploadUserFile, and from the later recreate-and-ensureSandboxInitialized path after SandboxNotAvailableError, never invoke the callback, so those P1 sandbox init errors are not sent to Sentry.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 7dd42b6. Configure here.
| tags: { module: 'TrueFoundryServiceFoundryServerClient', operation: 'requestJson' }, | ||
| extra: { url: input.url.href, method: input.method, status: response.status }, | ||
| }); | ||
| throw error; |
There was a problem hiding this comment.
SFY transport failures skip Sentry
Medium Severity
captureCriticalException runs only after a non-OK HTTP response (and non-JSON bodies), not when the ServiceFoundry fetch times out or fails to connect. Those transport errors still throw a 500 HTTPException and never reach Sentry, so the actual SFY hard failures are missing from P1 reporting.
Reviewed by Cursor Bugbot for commit 7dd42b6. Configure here.
| ## Auth server for Sentry DSN lookup when SENTRY_ENABLED=true in TrueFoundry mode. | ||
| # TRUEFOUNDRY_AUTH_SERVER_URL=https://auth.truefoundry.com | ||
| ## Optional tenantName query param for the auth-server Sentry DSN lookup. | ||
| # TRUEFOUNDRY_TENANT_NAME= |
There was a problem hiding this comment.
do we need this? we have tenant name env var already, can reuse?
| logger.warn(`Drain timed out after ${String(configuration.GRACEFUL_TIMEOUT_SECONDS)}s, exiting`); | ||
| process.exit(1); | ||
| captureCriticalException(new Error('Server drain timed out'), { | ||
| tags: { module: 'main', operation: 'drain' }, |
There was a problem hiding this comment.
how does module show up on sentry? Will we be able to filter by service: TrueForge? or should that be the module?
| console.error('Failed to start server:', error instanceof Error ? error.message : error); | ||
| process.exit(1); | ||
| captureCriticalException(error, { tags: { module: 'main', operation: 'listen' } }); | ||
| void exitAfterFlushSentry(1); |
| ({ sandboxInfo, sandboxCreated } = await this.ensureReadySandbox()); | ||
| } catch (e) { | ||
| this.logger.error('Sandbox initialization failed', extractErrorLogFields(e)); | ||
| this.onInitFailure?.(e); |
| this.requestTimeoutSeconds = Math.ceil(mcpBoundTimeoutMs / 1000) + NATS_REQUEST_TIMEOUT_BUFFER_SECONDS; | ||
| this.logger = options.logger.child({ module: 'Sandbox' }); | ||
| this.resolvedGitCredentialsContent = options.resolvedGitCredentialsContent ?? null; | ||
| this.onInitFailure = options.onInitFailure; |
There was a problem hiding this comment.
why do we need to take this as option ? sandbox method caller can alert if needed
| run_id: run.id, | ||
| error, | ||
| }); | ||
| captureCriticalException(error, { |
There was a problem hiding this comment.
why are we not doing the same at line 314. either lets capture all scenarios or none
| ...(isTrueFoundryModeEnabled() | ||
| ? { | ||
| priority: 'p1', | ||
| team: 'agent-team', |
There was a problem hiding this comment.
we cannot hard code these. Tags can be part of sentry config.
| import * as Sentry from '@sentry/node'; | ||
|
|
||
| export { captureCriticalException } from './captureCriticalException'; | ||
| export { initSentry, type InitSentryOptions } from './initSentry'; |
There was a problem hiding this comment.
do we want to init always? what if users don't want to enable sentry ?
2 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
Medium Risk
Touches process lifecycle (init/flush on exit) and outbound auth-server fetch for DSN; misconfiguration skips Sentry rather than blocking boot, but new captures may increase P1 noise if Sentry is enabled broadly.
Overview
Adds optional Sentry error reporting (off by default via
SENTRY_ENABLED) for the server and dedicated controller, with no-op behavior in development/test/localNODE_ENV.Initialization loads
@sentry/nodeat startup: in TrueFoundry mode it pulls init options from the auth server (TRUEFOUNDRY_AUTH_SERVER_URL+ API key, optionalTRUEFOUNDRY_TENANT_NAME); otherwise it requiresSENTRY_DSN. Fatal exit paths flush Sentry beforeprocess.exit.P1 captures use
captureCriticalException, taggingpriority: p1andteam: agent-teamwhen TrueFoundry mode is on, at controller loop failures, schedule dispatch handoff/processing errors, server boot/listen/drain failures, ServiceFoundry client hard failures, agent store dual-write cleanup failures, missing agentexternal_id, and sandbox init failures (new optionalonInitFailurehook on coreSandbox).Config and
.env.exampledocument the new variables; changeset bumps@truefoundry/trueforgeminor and@truefoundry/trueforge-corepatch.Reviewed by Cursor Bugbot for commit 7dd42b6. Bugbot is set up for automated code reviews on this repo. Configure here.