Skip to content

Commit aca152c

Browse files
icecrasher321icecrasher321
authored
fix(env): put runtime config on <html> so client reads can't outrun it (#6923)
* fix(env): put runtime config on <html> so client reads can't outrun it The inline script that assigns `window.__ENV` is rendered from the component tree, so it lands ~13KB after the `<script async>` bootstrap tags React emits in the preamble. `appBootstrap` calls `hydrate()` synchronously whenever `self.__next_s` is empty — which it always is now that the script is a plain tag rather than a `beforeInteractive` one, that queue having been the only thing sequencing the assignment ahead of hydration. So module bodies and the first commit could both read env before the assignment landed: the socket URL fell back to the page origin for the life of the document, `getBaseUrl()` threw, and every module-scope flag in `env-flags` froze on nothing. Carry the same snapshot on `<html>`, the document's first tag, and read it in `getEnv` when `window.__ENV` is not yet assigned. Parsing is memoized against the raw attribute rather than against having run once, so the cache can never serve a value the document no longer carries. `window.__ENV` stays the public global and the preferred read, and both transports are built from one function so they cannot drift. Alongside: guard the read-only webhook-URL field so a base URL it cannot resolve is a blank field rather than a dead canvas; report what the workflow error boundary catches, which it previously swallowed entirely; and enable PostHog's native exception capture, since error boundaries only ever see their own subtree and chunk-load failures, rejected promises and throws from event or socket callbacks reached nothing. * fix(realtime): count each failed connect attempt once `manager.reconnect()` calls `open()`, whose error path emits `error` — which the socket re-emits as `connect_error` — and then emits `reconnect_error` itself. A failed reconnect therefore reached both handlers and advanced the counter twice, so the outage report tripped on the second real attempt while claiming three. Count in `connect_error` alone: it is the only handler that fires exactly once for both the initial failure and every retry. `reconnect_error` keeps its log line and states why it deliberately does not count. * fix(workflow): let an unresolvable webhook URL fail loudly Reverts the guard added earlier in this PR. A blank row titled "Webhook URL" is a worse outcome than a crash: it explains nothing, and the value is one a user copies into a third-party provider, so any substitute — a guessed page origin, an empty string — is a URL that provider accepts and then never delivers to. The read can no longer come back empty from the hydration race this PR fixes, so reaching it at all means the deployment has no application base URL, which breaks webhook registration and callbacks regardless. The error boundary now reports what it caught, so the throw names its own cause instead of surfacing as an unexplained fallback.
1 parent 4ad1d53 commit aca152c

11 files changed

Lines changed: 394 additions & 47 deletions

File tree

‎apps/sim/app/_shell/providers/posthog-provider.tsx‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,23 @@ export function PostHogProvider({ children }: { children: React.ReactNode }) {
3535
capture_performance: false,
3636
capture_dead_clicks: false,
3737
enable_heatmaps: false,
38+
/**
39+
* PostHog's own error tracking, wired to `window.onerror` and
40+
* `unhandledrejection`. This is the app-wide net: React error
41+
* boundaries only see errors thrown inside the tree they wrap, and
42+
* a failed chunk load, a rejected promise, or anything thrown from
43+
* an event handler or socket callback reaches none of them.
44+
*
45+
* `capture_console_errors` stays off. It is not error reporting —
46+
* it captures every `console.error`, which here means React's
47+
* hydration and dev warnings (the ones `HydrationErrorHandler`
48+
* already filters out as noise) drowning the real exceptions.
49+
*/
50+
capture_exceptions: {
51+
capture_unhandled_errors: true,
52+
capture_unhandled_rejections: true,
53+
capture_console_errors: false,
54+
},
3855
disable_session_recording: true,
3956
session_recording: {
4057
maskAllInputs: false,

‎apps/sim/app/_shell/public-env-script.test.tsx‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22
* @vitest-environment node
33
*/
44
import { renderToStaticMarkup } from 'react-dom/server'
5-
import { describe, expect, it } from 'vitest'
6-
import { PublicEnvScript } from '@/app/_shell/public-env-script'
5+
import { describe, expect, it, vi } from 'vitest'
6+
import { PUBLIC_ENV_ATTRIBUTE } from '@/lib/core/config/env'
7+
import { PublicEnvScript, publicEnvHtmlAttributes } from '@/app/_shell/public-env-script'
8+
9+
vi.unmock('@/lib/core/config/env')
710

811
/**
912
* Guards the one property that matters: the emitted tag assigns `window.__ENV`
@@ -32,3 +35,35 @@ describe('PublicEnvScript', () => {
3235
expect(keys.every((key) => /^NEXT_PUBLIC_/i.test(key))).toBe(true)
3336
})
3437
})
38+
39+
/**
40+
* The script above is rendered from the component tree, so it lands at the end
41+
* of `<head>` - after the bootstrap chunks that can already be executing. These
42+
* attributes go on `<html>`, the document's first tag, which is what makes the
43+
* same values readable by code that runs in that gap.
44+
*/
45+
describe('publicEnvHtmlAttributes', () => {
46+
it('carries the public env under the attribute getEnv reads', () => {
47+
const attributes = publicEnvHtmlAttributes()
48+
49+
expect(Object.keys(attributes)).toEqual([PUBLIC_ENV_ATTRIBUTE])
50+
expect(() => JSON.parse(attributes[PUBLIC_ENV_ATTRIBUTE])).not.toThrow()
51+
})
52+
53+
it('exposes only NEXT_PUBLIC_ variables', () => {
54+
const values = JSON.parse(publicEnvHtmlAttributes()[PUBLIC_ENV_ATTRIBUTE])
55+
56+
expect(Object.keys(values).every((key) => /^NEXT_PUBLIC_/i.test(key))).toBe(true)
57+
})
58+
59+
/**
60+
* Two transports for one snapshot only stays safe while they agree; a reader
61+
* that resolved different values depending on which one it happened to hit
62+
* would be worse than the race this replaces.
63+
*/
64+
it('carries exactly what the script assigns', () => {
65+
const values = JSON.parse(publicEnvHtmlAttributes()[PUBLIC_ENV_ATTRIBUTE])
66+
67+
expect(values).toEqual(PublicEnvScript().props.env)
68+
})
69+
})

‎apps/sim/app/_shell/public-env-script.tsx‎

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,48 @@
11
import { EnvScript } from 'next-runtime-env'
2+
import { PUBLIC_ENV_ATTRIBUTE } from '@/lib/core/config/env'
3+
4+
/**
5+
* Every `NEXT_PUBLIC_*` value currently in `process.env`. Filter matches
6+
* `next-runtime-env`'s own `getPublicEnv()` exactly.
7+
*/
8+
function readPublicEnv(): Record<string, string | undefined> {
9+
return Object.fromEntries(
10+
Object.entries(process.env).filter(([key]) => /^NEXT_PUBLIC_/i.test(key))
11+
)
12+
}
213

314
/**
415
* `NEXT_PUBLIC_*` values, captured once when this module is first loaded - i.e.
516
* at server start on the hosted deployment, where a build's env never changes
6-
* between requests. Filter matches `next-runtime-env`'s own `getPublicEnv()`
7-
* exactly.
17+
* between requests (`bootstrap.ts` awaits the runtime secret before importing
18+
* the server, so `process.env` is complete before any module evaluates).
819
*
920
* These are deliberately NOT the values Next inlines into the client bundle:
1021
* the image is built with placeholder `NEXT_PUBLIC_*` values and the real ones
11-
* are supplied to the container at start, so `window.__ENV` is the only source
12-
* of truth in the browser.
22+
* are supplied to the container at start, so the browser has no compiled-in
23+
* copy to fall back on.
24+
*/
25+
const HOSTED_PUBLIC_ENV = readPublicEnv()
26+
27+
/**
28+
* Props to spread onto the `<html>` element so the public env is readable by any
29+
* client code that can run at all.
30+
*
31+
* The script below is rendered from the component tree and therefore lands at
32+
* the end of `<head>`, well after the `<script async>` bootstrap tags React
33+
* emits in the preamble — see {@link PUBLIC_ENV_ATTRIBUTE} for the full ordering
34+
* argument and why that gap is reachable. `<html>` is the document's first tag,
35+
* so its attributes are parsed before any script exists to read them.
36+
*
37+
* Read fresh rather than from {@link HOSTED_PUBLIC_ENV} so the one helper serves
38+
* both deployment modes: self-hosted images re-inject env per deploy without a
39+
* rebuild, and `next-runtime-env`'s script reads `process.env` per request for
40+
* exactly that reason. On hosted the two reads are the same values, because
41+
* nothing mutates `process.env` after boot.
1342
*/
14-
const HOSTED_PUBLIC_ENV = Object.fromEntries(
15-
Object.entries(process.env).filter(([key]) => /^NEXT_PUBLIC_/i.test(key))
16-
)
43+
export function publicEnvHtmlAttributes(): Record<string, string> {
44+
return { [PUBLIC_ENV_ATTRIBUTE]: JSON.stringify(readPublicEnv()) }
45+
}
1746

1847
/**
1948
* Static equivalent of `next-runtime-env`'s `<PublicEnvScript>` for the hosted
@@ -35,13 +64,13 @@ const HOSTED_PUBLIC_ENV = Object.fromEntries(
3564
* `window.__ENV` stays undefined for the entire lifetime of the document -
3665
* every `getEnv` read empty, until a reload happens to win the race.
3766
*
38-
* A plain `<script>` assigns unconditionally when the parser reaches it. When
39-
* it is reached before the bootstrap chunk runs it lands strictly earlier than
40-
* the queue drain would have; when it is not, the value still arrives a few
41-
* milliseconds late instead of never. There is no supported way to place an
42-
* inline script ahead of the framework's own bootstrap tags - React emits those
43-
* in the preamble, before any content from the component tree - so the goal is
44-
* to make losing that race harmless rather than to try to win it.
67+
* A plain `<script>` assigns unconditionally when the parser reaches it, so a
68+
* lost race costs milliseconds instead of the session. It does not make the
69+
* assignment win the race, though: draining that queue was also the only thing
70+
* sequencing the assignment ahead of `hydrate()`, and with the queue empty
71+
* `appBootstrap` hydrates synchronously. {@link publicEnvHtmlAttributes} is what
72+
* closes the remaining window - this tag stays because `window.__ENV` is the
73+
* documented global, and it is what `getEnv` reads first.
4574
*/
4675
export function PublicEnvScript() {
4776
return <EnvScript env={HOSTED_PUBLIC_ENV} disableNextScript />

‎apps/sim/app/layout.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { QueryProvider } from '@/app/_shell/providers/query-provider'
1919
import { SessionProvider } from '@/app/_shell/providers/session-provider'
2020
import { ThemeProvider } from '@/app/_shell/providers/theme-provider'
2121
import { TooltipProvider } from '@/app/_shell/providers/tooltip-provider'
22-
import { PublicEnvScript } from '@/app/_shell/public-env-script'
22+
import { PublicEnvScript, publicEnvHtmlAttributes } from '@/app/_shell/public-env-script'
2323
import { season } from '@/app/_styles/fonts/season/season'
2424

2525
export const viewport: Viewport = {
@@ -40,7 +40,7 @@ export default function RootLayout({ children }: { children: React.ReactNode })
4040
const themeCSS = generateThemeCSS()
4141

4242
return (
43-
<html lang='en' suppressHydrationWarning>
43+
<html lang='en' suppressHydrationWarning {...publicEnvHtmlAttributes()}>
4444
<head>
4545
{isReactScanEnabled && (
4646
<Script

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/error/index.tsx‎

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
'use client'
22

3-
import { Component, type ReactNode, useEffect } from 'react'
3+
import { Component, type ErrorInfo, type ReactNode } from 'react'
44
import { Button } from '@sim/emcn'
55
import { RefreshCw } from '@sim/emcn/icons'
66
import { createLogger } from '@sim/logger'
7+
import { truncate } from '@sim/utils/string'
78
import { ReactFlowProvider } from 'reactflow'
9+
import { captureClientEvent } from '@/lib/posthog/client'
810
import { Panel } from '@/app/workspace/[workspaceId]/w/[workflowId]/components'
911
import { usePreventZoom } from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks'
1012
import { Sidebar } from '@/app/workspace/[workspaceId]/w/components/sidebar/sidebar'
1113
import { readCollapsedCookie } from '@/stores/sidebar/store'
1214

1315
const logger = createLogger('ErrorBoundary')
1416

17+
/** Keeps a runaway stack out of the event payload without losing the top frames. */
18+
const MAX_REPORTED_COMPONENT_STACK = 2000
19+
1520
/**
1621
* Shared Error UI Component
1722
*/
@@ -90,6 +95,33 @@ export class ErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundarySt
9095
return { hasError: true, error }
9196
}
9297

98+
/**
99+
* Reports what was caught. This boundary latches for the life of the document
100+
* and its fallback names nothing, so without this the only trace of a canvas
101+
* crash is React's own console output on whichever machine happened to hit it
102+
* — leaving an intermittent failure with no evidence to diagnose from.
103+
* `error.name` is carried separately from the message because it is what
104+
* separates the failure classes from each other.
105+
*/
106+
public componentDidCatch(error: Error, errorInfo: ErrorInfo) {
107+
const componentStack = errorInfo.componentStack ?? undefined
108+
109+
logger.error('Workflow canvas crashed', {
110+
name: error.name,
111+
message: error.message,
112+
stack: error.stack,
113+
componentStack,
114+
})
115+
116+
captureClientEvent('workflow_canvas_crashed', {
117+
error_name: error.name,
118+
error_message: error.message,
119+
component_stack: componentStack
120+
? truncate(componentStack, MAX_REPORTED_COMPONENT_STACK)
121+
: undefined,
122+
})
123+
}
124+
93125
public render() {
94126
if (this.state.hasError) {
95127
return this.props.fallback || <ErrorUI />
@@ -98,20 +130,3 @@ export class ErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundarySt
98130
return this.props.children
99131
}
100132
}
101-
102-
/**
103-
* Next.js Error Page Component
104-
* Renders when a workflow-specific error occurs
105-
*/
106-
interface NextErrorProps {
107-
error: Error & { digest?: string }
108-
reset: () => void
109-
}
110-
111-
export function NextError({ error, reset }: NextErrorProps) {
112-
useEffect(() => {
113-
logger.error('Workflow error:', { error })
114-
}, [error])
115-
116-
return <ErrorUI onReset={reset} />
117-
}

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,12 @@ const SubBlockRow = memo(function SubBlockRow({
500500
if (!subBlock?.id?.startsWith('webhookUrlDisplay') || !blockId) {
501501
return null
502502
}
503+
/* Deliberately unguarded. `getBaseUrl` throws when no application base URL is
504+
configured, and that is the right outcome here: this value gets copied into
505+
a third-party provider, so a guessed origin would hand the user a URL that
506+
provider accepts and then never delivers to, and a blank row explains
507+
nothing. The error boundary reports what it caught, so the throw names its
508+
own cause. */
503509
const baseUrl = getBaseUrl()
504510
const triggerPath = allSubBlockValues?.triggerPath?.value as string | undefined
505511
return triggerPath

‎apps/sim/app/workspace/providers/socket-provider.tsx‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ import { backoffWithJitter } from '@sim/utils/retry'
3030
import { useQueryClient } from '@tanstack/react-query'
3131
import { useParams } from 'next/navigation'
3232
import type { Socket } from 'socket.io-client'
33+
import { getEnv } from '@/lib/core/config/env'
3334
import { getSocketUrl } from '@/lib/core/utils/urls'
35+
import { captureClientEvent } from '@/lib/posthog/client'
3436
import {
3537
type SocketJoinCommand,
3638
SocketJoinController,
@@ -54,6 +56,14 @@ const logger = createLogger('SocketContext')
5456

5557
const TAB_SESSION_ID_KEY = 'sim_tab_session_id'
5658

59+
/**
60+
* Consecutive connect failures before the realtime connection is reported as
61+
* failing. Three attempts at the 1s base delay lands around the same few seconds
62+
* as the "Reconnecting…" toast, so the event marks a real outage rather than the
63+
* sub-second transport hiccups that recover on the first retry.
64+
*/
65+
const CONNECT_FAILURES_BEFORE_REPORT = 3
66+
5767
/** Bounded auto-retry budget for auth-class connect failures before going terminal. */
5868
const MAX_AUTH_RETRY_ATTEMPTS = 5
5969
const AUTH_RETRY_BASE_MS = 1000
@@ -174,6 +184,8 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
174184
const authRetryAttemptsRef = useRef(0)
175185
const authRetryTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null)
176186
const sessionRejectedRef = useRef(false)
187+
const connectFailureCountRef = useRef(0)
188+
const connectFailureReportedRef = useRef(false)
177189
const queryClient = useQueryClient()
178190

179191
const params = useParams()
@@ -375,6 +387,15 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
375387
try {
376388
const { io } = await import('socket.io-client')
377389
const socketUrl = getSocketUrl()
390+
/* Origin only: enough to tell a misresolved host from a down service,
391+
without putting a path or query into an analytics payload. `new URL`
392+
rather than `URL.parse`, which is unavailable on Safari below 18. */
393+
let socketOrigin = 'unparseable'
394+
try {
395+
socketOrigin = new URL(socketUrl).origin
396+
} catch {
397+
/* Reported as-is; an unparseable socket URL is itself the finding. */
398+
}
378399

379400
logger.info('Attempting to connect to Socket.IO server', {
380401
url: socketUrl,
@@ -409,11 +430,52 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
409430
},
410431
})
411432

433+
/**
434+
* Reports a realtime connection that is not coming back.
435+
*
436+
* A socket that never connects raises no exception anywhere — every
437+
* failure path here is handled, so error tracking sees nothing and the
438+
* only user-visible trace is a "Reconnecting…" toast. Socket.IO then
439+
* retries the same URL forever, so the failure is both permanent and
440+
* silent. This is the one signal that distinguishes "the realtime
441+
* service is down" from "this client resolved the wrong URL", which is
442+
* why the origin is reported alongside the reason.
443+
*
444+
* Fires at most once per socket instance, at the point the toast
445+
* appears, rather than once per retry.
446+
*
447+
* Called from `connect_error` and nowhere else. That is the only handler
448+
* that sees exactly one event per attempt: a failed *reconnect* also
449+
* emits the manager's `reconnect_error` (`manager.reconnect()` calls
450+
* `open()`, whose error path emits `error` — which the socket re-emits as
451+
* `connect_error` — and then emits `reconnect_error` itself), so counting
452+
* in both would advance twice per try and trip the threshold early.
453+
*/
454+
const reportPersistentConnectFailure = (reason: string) => {
455+
connectFailureCountRef.current += 1
456+
if (
457+
connectFailureReportedRef.current ||
458+
connectFailureCountRef.current < CONNECT_FAILURES_BEFORE_REPORT
459+
) {
460+
return
461+
}
462+
connectFailureReportedRef.current = true
463+
464+
captureClientEvent('realtime_connection_failing', {
465+
socket_origin: socketOrigin,
466+
expected_socket_origin_configured: Boolean(getEnv('NEXT_PUBLIC_SOCKET_URL')?.trim()),
467+
attempts: connectFailureCountRef.current,
468+
reason,
469+
})
470+
}
471+
412472
socketInstance.on('connect', () => {
413473
setIsConnected(true)
414474
setIsConnecting(false)
415475
setIsReconnecting(false)
416476
authRetryAttemptsRef.current = 0
477+
connectFailureCountRef.current = 0
478+
connectFailureReportedRef.current = false
417479
clearAuthRetryTimeout()
418480
setCurrentSocketId(socketInstance.id ?? null)
419481
logger.info('Socket connected successfully', {
@@ -451,6 +513,7 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
451513
message: error.message,
452514
})
453515
setIsReconnecting(true)
516+
reportPersistentConnectFailure(error.message)
454517
return
455518
}
456519

@@ -518,6 +581,8 @@ export function SocketProvider({ children, user }: SocketProviderProps) {
518581
logger.info('Socket reconnection attempt', { attemptNumber })
519582
})
520583

584+
/* Deliberately does not count toward the outage report — the socket's
585+
own `connect_error` already fired for this same attempt. */
521586
socketInstance.io.on('reconnect_error', (error: Error) => {
522587
logger.warn('Socket reconnection attempt failed, will retry', {
523588
message: error.message,

0 commit comments

Comments
 (0)