Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/desktop/scripts/ensure-pty-prebuilds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
import { execFileSync } from 'node:child_process'
import { createHash, timingSafeEqual } from 'node:crypto'
import {
cpSync,
existsSync,
mkdirSync,
mkdtempSync,
readFileSync,
renameSync,
rmSync,
writeFileSync,
} from 'node:fs'
Expand Down Expand Up @@ -103,7 +103,7 @@ async function fetchPrebuild(arch: string, version: string): Promise<void> {
const target = packageDir(arch)
mkdirSync(dirname(target), { recursive: true })
rmSync(target, { recursive: true, force: true })
renameSync(join(staging, 'package'), target)
cpSync(join(staging, 'package'), target, { recursive: true, force: true })
Comment on lines 105 to +106

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Copy Can Leave Partial Install

The script now deletes the working target and copies directly into that live directory, so interruption, disk exhaustion, or a copy error can leave a partial installation. A later run checks only whether pty.node exists, so a torn copy containing that file can be treated as valid and reused. Copy into a temporary directory on the destination filesystem first, then atomically rename it into place.

} finally {
rmSync(staging, { recursive: true, force: true })
}
Expand Down
1 change: 1 addition & 0 deletions apps/sim/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ CRON_SECRET=your_cron_secret # Use `openssl rand -hex 32` to generate. Authentic
# LITELLM_BASE_URL=http://localhost:4000 # Base URL for your LiteLLM proxy (OpenAI-compatible)
# LITELLM_API_KEY= # Optional bearer token if your LiteLLM proxy requires auth
# OPENROUTER_API_KEY= # Optional self-hosted fallback for OpenAI knowledge-base embeddings
# DISABLE_TELEMETRY=1 # Disable all OpenTelemetry collection (traces, metrics, logs). Prevents export timeouts when the telemetry endpoint is unreachable
# NEXT_PUBLIC_FORCE_HOSTED=true # Dev only: treat this instance as hosted Sim (sim-auto pool, platform keys); ignored in production builds
# FIREWORKS_API_KEY= # Optional Fireworks AI API key for model listing and inference
# FIREWORKS_API_KEY_1= # Optional Fireworks API key for rotation (hosted deployments)
Expand Down
24 changes: 16 additions & 8 deletions apps/sim/instrumentation-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const DEFAULT_TELEMETRY_CONFIG = {
maxQueueSize: 2048,
maxExportBatchSize: 512,
scheduledDelayMillis: 5000,
exportTimeoutMillis: 30000,
exportTimeoutMillis: 10000,
},
}

Expand Down Expand Up @@ -151,8 +151,12 @@ class MothershipOriginSpanProcessor implements SpanProcessor {

async function initializeOpenTelemetry() {
try {
if (env.NEXT_TELEMETRY_DISABLED === '1' || process.env.NEXT_TELEMETRY_DISABLED === '1') {
logger.info('OpenTelemetry disabled via NEXT_TELEMETRY_DISABLED=1')
if (
process.env.DISABLE_TELEMETRY === '1' ||
env.NEXT_TELEMETRY_DISABLED === '1' ||
process.env.NEXT_TELEMETRY_DISABLED === '1'
) {
logger.info('OpenTelemetry disabled via env var')
return
}

Expand Down Expand Up @@ -359,15 +363,19 @@ async function initializeOpenTelemetry() {

const shutdownOtel = async () => {
try {
await sdk.shutdown()
const shutdownPromise = sdk.shutdown()
const timeoutPromise = new Promise<void>((_, reject) =>
setTimeout(() => reject(new Error('OTel shutdown timed out')), 5000)
)
Comment on lines +367 to +369

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Shared Delay Helper Bypassed

This change constructs a delay with new Promise and setTimeout, violating the repository directive to use the shared sleep(ms) helper from @sim/utils/helpers instead of inline timeout promises. This repository requirement must be satisfied before merging.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

await Promise.race([shutdownPromise, timeoutPromise])
Comment on lines +367 to +370

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Timeout Delays Successful Shutdown

When sdk.shutdown() finishes first, this five-second timer is neither cleared nor unreferenced. The installed SIGTERM and SIGINT listeners disable Node's default signal exit, so the pending timer keeps the event loop alive for the rest of those five seconds even after shutdown succeeds. Store and clear the timer in a finally block, or otherwise prevent it from keeping the process alive.

logger.info('OpenTelemetry SDK shut down successfully')
} catch (err) {
logger.error('Error shutting down OpenTelemetry SDK', err)
}
}

process.on('SIGTERM', shutdownOtel)
process.on('SIGINT', shutdownOtel)
process.once('SIGTERM', shutdownOtel)
process.once('SIGINT', shutdownOtel)
Comment on lines +377 to +378

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Handlers Still Accumulate

process.once() removes a listener only after it fires; it does not deduplicate listeners added by repeated instrumentation registration during HMR. Each reload therefore adds another OTel shutdown closure here and another PostHog closure at lines 409–410. All accumulated handlers run on the next signal, so duplicate shutdowns still occur. Make initialization idempotent or explicitly replace the previous listeners.


logger.info('OpenTelemetry instrumentation initialized', {
serviceName: telemetryConfig.serviceName,
Expand Down Expand Up @@ -398,8 +406,8 @@ export async function register() {
}
}

process.on('SIGTERM', shutdownPostHog)
process.on('SIGINT', shutdownPostHog)
process.once('SIGTERM', shutdownPostHog)
process.once('SIGINT', shutdownPostHog)

const { startMemoryTelemetry } = await import('./lib/monitoring/memory-telemetry')
startMemoryTelemetry()
Expand Down
18 changes: 18 additions & 0 deletions apps/sim/lib/auth/anonymous.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,19 @@ const logger = createLogger('AnonymousAuth')

let anonymousUserEnsured = false

function isMissingTableError(error: unknown): boolean {
if (error && typeof error === 'object') {
const code = (error as Record<string, unknown>).code
if (code === '42P01') return true
const cause = (error as Record<string, unknown>).cause
if (cause && typeof cause === 'object') {
const innerCode = (cause as Record<string, unknown>).code
if (innerCode === '42P01') return true
}
}
Comment on lines +18 to +21

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Nested Errors Remain Undetected

This checks only the top-level error and its immediate cause, although the repository's PostgreSQL error handling supports deeper cause chains. If Drizzle wraps 42P01 beneath two causes, this helper misses it and self-hosters still get the generic failure instead of the migration guidance. Reuse getPostgresErrorCode from @sim/utils, which already walks nested causes safely.

return false
}

/**
* Ensures the anonymous user and their stats record exist in the database.
* Called when DISABLE_AUTH is enabled to ensure DB operations work.
Expand Down Expand Up @@ -47,6 +60,11 @@ export async function ensureAnonymousUserExists(): Promise<void> {

anonymousUserEnsured = true
} catch (error) {
if (isMissingTableError(error)) {
throw new Error(
'Database tables not found. Run database migrations before starting the app: bun run db:migrate'
)
}
if (
error instanceof Error &&
(error.message.includes('unique') || error.message.includes('duplicate'))
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/telemetry.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const config = {
maxQueueSize: 2048,
maxExportBatchSize: 512,
scheduledDelayMillis: 5000,
exportTimeoutMillis: 30000,
exportTimeoutMillis: 10000,
},

/**
Expand Down