Implement MAX_EVENTS_PER_RUN limit#2986
Conversation
🦋 Changeset detectedLatest commit: 78308f6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 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 |
📊 Workflow Benchmarks❌ The benchmark run for commit Backend:
📜 Previous results (1)330a548Fri, 17 Jul 2026 16:29:55 GMT · run logs
Avg deltas compare against the most recent benchmark run on Metrics — TTFS: time to first step body (in-deployment start() → first step body, deployment clocks) · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (whole-run time outside step bodies, in-deployment anchored) · SL: stream latency (in-deployment write → read propagation, readAt - writtenAt) Scenarios — stream: one step that streams chunks back to the client; no hooks, so the run stays in turbo mode · hook + stream: registers a hook before the same streaming step, which exits turbo mode · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges 🟢/🔴 mark percentiles within/above target. Targets (p75/p90/p99, ms) — TTFS 200/300/600 · SL 50/60/125 · STSO (1-20) 20/30/60 · STSO (101-120) 30/45/90 · STSO (1001-1020) 40/60/120 All metrics are measured from deployment-side timestamps only. Runs are triggered by an in-deployment route that stamps the anchor ( Cold starts are kept in the numbers on purpose — they are part of real bursty-workload latency. The workbench deployment cold-starts the |
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
✅ vercel-multi-region
|
4d4aefc to
d4ddf7e
Compare
d4ddf7e to
330a548
Compare
| ); | ||
| } catch (err) { | ||
| // Best-effort: the run stops anyway when the loop returns; log why. | ||
| runLogger.warn('Unable to mark run as failed after exceeding event limit', { |
There was a problem hiding this comment.
This might be a problem: if it fails, and then the runtime returns, the workflow is stuck in "active" for unknown reasons. Let me double check what happens when we fail to create run_failed in a regular failure scenario
| */ | ||
| export const MAX_EVENTS_PER_RUN = 25_000; | ||
|
|
||
| export const MAX_EVENTS_BUFFER = 100; |
There was a problem hiding this comment.
The buffer would only be needed world-side, as a safeguard in case the client side fails to terminate correctly. We can remove the buffer from this PR
VaguelySerious
left a comment
VaguelySerious
left a comment
There was a problem hiding this comment.
Reviewed the diff and traced how it sits in the replay loop and the existing terminal-error machinery. Design is sound — the guard is correctly placed after the hasRecordedTerminalRunEvent check and before runWorkflow, inside the try at runtime.ts:1164 (catch at 1489), so it can't fire on an already-terminal run and re-checks every loop iteration. The "deterministic → no retry, no process.exit" rationale is right. A few suggestions below, the first being the main one.
Main suggestion: throw a dedicated error and let the existing catch write run_failed
The guard lives inside the try whose catch (line 1489) already funnels non-suspension, non-retryable errors into the terminal run_failed writer (~2440): classifyRunError → dehydrateRunError → world.events.create({eventType:'run_failed'}), with EntityConflictError/RunExpiredError-already-finished handling, turbo awaitRunReady(), and span/stack remapping all built in.
I traced a thrown error through it: not a WorkflowSuspension, not PreconditionFailedError, isRetryableWorldError → false (so no risk of false redelivery/loop), so it writes run_failed and returns. That means the whole bespoke event-limit.ts create path is duplicating machinery the catch already owns.
The one catch: classifyRunError (classify-error.ts:110) has explicit branches for ReplayDivergenceError and CorruptedEventLogError, then falls through to USER_ERROR. A plain FatalError would be recorded as USER_ERROR, silently dropping the new MAX_EVENTS_EXCEEDED code (and the MAX_EVENTS_HINT would never fire). So the idiomatic move — matching the two precedents already in that function — is:
- Add a
MaxEventsExceededErrorclass. - Add one branch to
classifyRunErrorreturningRUN_ERROR_CODES.MAX_EVENTS_EXCEEDED. - Throw it from the guard instead of calling
handleEventLimitExceeded.
That deletes ~40 lines of duplicated run_failed plumbing, inherits the catch block's already-terminal / turbo / stack-remap handling (i.e. finding #1 and #4 below come for free), and keeps the distinct error code + attribution. The current self-contained handler is defensible for explicitness, but it re-solves problems the terminal path already solves.
Other findings (all non-blocking)
-
Best-effort catch conflates "already terminal" with real failure. The sibling
MAX_DELIVERIEShandler (runtime.ts:420) special-casesEntityConflictError/RunExpiredErrorand returns silently, only logging the loud message for genuine backend failures.handleEventLimitExceededfunnels everything into onewarn('Unable to mark run as failed…'), so a concurrent handler terminating the run between the guard's terminal check and thisevents.createyields a misleading warning. Worth mirroring the sibling's discrimination (moot if you adopt the throw approach — the catch already does this). -
getMaxEventsPerRun()is called twice (once formaxEvents, once forlimit:). Compute once. -
limitpassed to the handler excludes the buffer, so the message reads "maximum of 25000 events" while the run actually reached ~25100 (eventCountis the real count). Probably intended since 25k is the published number — worth a one-line comment so it doesn't read as an off-by-buffer bug. -
No
awaitRunReady()before the write, unlike every otherrun_failedwriter here. Fine in practice — a run with >25k events necessarily has a durablerun_started, so the turbo backgrounded-run_startedordering concern doesn't apply. (Also comes for free with the throw approach.)
Nit confirmed fine: dropping dehydrateRunError's compression arg matches the MAX_DELIVERIES sibling, so that's consistent.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
2 participants
Summary
Enforces a server provided limit for events-per-run. We publish a limit that that was previously not enforced. Separately a server change will return the limit. Once a run's event log exceeds the limit, the runtime fails it with a new
MAX_EVENTS_EXCEEDEDcode instead of letting a runaway workflow (e.g. an unbounded step loop) grow the event log without bound.Checked at the top of the replay loop so it fails without retry and is attributed to user code.
Testing
Unit tests for the handler, the limit constant/override, and the error attribution
pnpm exec vitest run src/runtime/event-limit.test.ts src/describe-error.test.ts