feat(execution): classify reference executor errors as permanent or transient - #130
Open
dawidaksamski wants to merge 12 commits into
Open
feat(execution): classify reference executor errors as permanent or transient#130dawidaksamski wants to merge 12 commits into
dawidaksamski wants to merge 12 commits into
Conversation
resolveTemplate threw plain errors, so a malformed or unresolved reference was retried under the node profile even though a retried node receives the same context and fails identically. The three throw sites now raise PermanentNodeExecutionError with the codes template_malformed and template_unresolved, so the engine stops on the first attempt and the code reaches node_failed.
A decision node with no matching branch was retried under the node profile, although the same inputs yield the same non-match on every attempt. The throw site now raises PermanentNodeExecutionError, so the node stops on its first attempt and the no_branch_matched code reaches node_failed for the first time: unclassified codes are dropped at the activity boundary.
The AI agent rethrew whatever the AI SDK threw, so a rejected API key or a 400 burned every attempt the node profile allows. The catch block now maps a provider response by status, at the throw site and nowhere central: 401/403 and every other 4xx are permanent, 429, 5xx, 408 and a request that never got an answer are transient. Anything that is not a provider response passes through unclassified. The provider's own error stays attached as the cause, so node_failed keeps showing the provider's text; the HTTP status is in the message. The log line gains the code. The worker README documents the table.
The boundary test proved the permanent and unclassified paths but not the transient one. The new case runs a wrapped transient failure through a real Temporal dev server and pins that the node retries to the profile's limit, that node_failed carries the code and the attempt it died on, and that the deepest cause's message is what reaches it.
…wrapper classifyProviderError only matched a bare APICallError, so with SDK retries enabled every provider failure would arrive as a RetryError and fall back to the profile's uniform retry. The classifier now unwraps lastError first. Also: the status-less branch is reached for connection failures, not for a request that timed out, so the message and README row say so; the README no longer claims every failure is classified; the activity log line uses instanceof instead of a cast; the decision comment fits the three-line ceiling; the unclassified boundary case no longer borrows the no_branch_matched code this branch made permanent.
extractDeepestError returned the deepest cause's message verbatim. A failed fetch in Node ends in an AggregateError with an empty message, so a provider that refused the connection reached node_failed, and the AI Studio log panel, as a blank line. The walk now keeps the deepest non-empty message; everything else about the chain is unchanged.
The template tests grew a bespoke catch-and-return helper next to twenty assertions written with toThrow; they now use toThrow with an asymmetric matcher like the rest of the file. The graph-runner test that models a template failure crossing an adapter builds the real PermanentNodeExecutionError and asserts the code it now carries. The comment above resolveTemplate restated the README and is gone.
…iberate choices The README claimed the HTTP status reaches node_failed. It does not: the event reports the deepest cause, so the provider's text is what the UI shows and the status lives only in Temporal's failure record. The paragraph now says so, states that 409 is permanent on purpose, and names the unparsable-2xx case as intentionally unclassified. Two comments that restated the READMEs or overstated the log line are trimmed to what the code cannot show.
…2xx fall-through The APICallError fixture was built in two test files; it now lives in one. The classifier table is a single list keyed by class, with the redundant classification literals dropped, and gains 409 plus the unparsable-2xx case that passes through unclassified. The comment about statusCode 500 moves back to the single-call test it describes, and the decision test asserts with toThrow matchers.
…catch-all The no_branch_matched message advised adding a catch-all branch with no conditions, but a branch with no conditions never matches, so the advice reproduced the failure it explained. The message now asks for an always-true condition, the executor comment says why, and the test pins the wording. Making an empty branch the catch-all is a separate backlog item, marked with the follow-up slug in the comment.
…branch The "How-to / Other" branch had no conditions, which the decision executor never matches, so any ticket classified as neither billing nor bug failed the run with no_branch_matched. The branch now carries an always-true condition, the only form the executor treats as a catch-all today.
…anch follow-up The slug presumed that an empty branch will become the catch-all. The backlog task proposes an explicit default flag instead, so the marker now names the outcome neutrally.
dawidaksamski
requested review from
librowski,
lukasz-jazwa,
piotrblaszczyk and
szymon-t-sc
as code owners
September 9, 2026 15:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Executors threw plain errors, so Temporal retried everything, including calls that fail identically every time: a rejected API key, a 400, a template typo, a decision with no matching branch. This is the reviewed classification pass over the reference executors. Every judgment is declared at the throw site; the runner and the adapter still never infer a class from a status code.
provider_auth_rejectedprovider_rejected_requestprovider_rate_limitedprovider_unavailableprovider_unreachabletemplate_malformed,template_unresolvedno_branch_matchedDeliberate choices, both documented in the worker README:
APICallError.statusCodewith its own table and never readsisRetryable, so an SDK upgrade cannot reclassify anything. 409 is permanent on purpose, unlike the SDK default.Changes by area
execution-core
resolveTemplatethrowsPermanentNodeExecutionErrorat its three throw sites. Messages are unchanged. A retried node receives the same context, so a bad reference can never resolve on attempt two.extractDeepestErrorkeeps the deepest non-empty message. Node's failed fetch ends in anAggregateErrorwith an empty message, which otherwise reachednode_failedand the AI Studio log panel as a blank line.execution-worker
classifyProviderErrornext to the AI agent activity, called from its catch block, with the table above. The provider's own error stays attached ascause, sonode_failedkeeps showing the provider's text; the classifier's message (which names the HTTP status) is visible in Temporal's failure record. Handles a provider error wrapped in the SDK'sRetryErrorin case client retries are ever re-enabled.no_branch_matchedbecomes permanent.no_branch_matchedmessage no longer recommends "a catch-all branch with no conditions", which the executor never matches. It now asks for an always-true condition. The proper default-branch mechanism is WB-619 (follow-up: decision-default-branch).ai-studio
temporal
node_failedcarries the code and attempt, and the deepest cause's text is what reaches it.Visible behaviour changes worth knowing
no_branch_matchedand the template codes reachnode_failedfor the first time. Unclassified codes were dropped at the Temporal boundary, so a consumer test asserting the old payload shape would break.Verification
pnpm testpasses in every workspace. execution-core, execution-worker and temporal are the ones touched.pnpm lintclean.pnpm typecheckfails only inapps/docs, which is the pre-existing failure onmain.No changeset: the worker is private and
@workflowbuilder/temporalis unpublished; its queued first-release changeset covers the execution-core changes.