fix(api): return 200 with embedded log on SAS session failure instead… - #392
Merged
Conversation
… of 400 A failed session (e.g. SAS %abort;, or a non-zero JS/PY/R exit) is a normal outcome of running arbitrary user code, not a request-shape or server problem. The #388 fix stopped processProgram() from hanging forever on a failed SAS session, but did so by throwing a SessionExecutionError, which surfaced as an HTTP 400 with a bespoke JSON shape - breaking Studio's log tab, which only renders on 2xx. Align the SAS branch with the pre-existing JS/PY/R behaviour: resolve instead of throwing, and let Execution.ts fold session.failureReason into the log the same way it already does for a debug-mode run. This removes the now-dead SessionExecutionError class and its try/catch wrapper entirely. Update the diagrams in api/docs/diagrams/ to match.
allanbowe
reviewed
Jul 15, 2026
| // successful run (200, same body shape, no hang), with the log simply | ||
| // reflecting what happened. Regression test for #388's follow-up: the | ||
| // original #388 fix stopped the hang but over-corrected into a 400 | ||
| // with a bespoke error shape, which broke Studio's log-tab rendering. |
Member
There was a problem hiding this comment.
can we add something to the CONTEXT.md to stop the AI writing comments that refer to previous tickets? They're useful for pull request reviews but the comment will be stale almost immediately
allanbowe
approved these changes
Jul 15, 2026
|
🎉 This PR is included in version 0.39.8 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
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.
Issue
Follow-up to #388. #388 stopped a SAS session failure (e.g.
%abort;) from hanging the request forever, but did so by throwing, which surfaced as an HTTP 400 with a bespoke JSON error shape. In production this broke the Studio frontend: instead of showing the log in the LOG tab like every other run, it popped up a raw-JSON "Abort" modal, because the frontend only parses/renders the log on a 2xx response.Intent
A failed session (SAS
%abort;, or a non-zero JS/PY/R exit) is a normal outcome of running arbitrary user code, not a request-shape or server problem. The response should:Implementation
processProgramwith the pre-existing JS/PY/R behaviour: it now resolves instead of throwing whensession.statebecomesfailed, just stopping its poll loop.Execution.ts'sSessionExecutionErrorclass and thetry/catchwrapper aroundprocessProgram()- both became dead code once the SAS branch stopped throwing.Execution.tsalready knew how to foldsession.failureReasoninto a normal 200 response (it's the same logic that already handled JS/PY/R failures and debug-mode logs); it just couldn't be reached for SAS before.logfield from thecode.ts/stp.tscatch blocks.mockSas.jsto write abort errors into the log file itself (not just stderr), matching real SAS behaviour, so tests assert on what the API actually returns.processProgram.spec.ts,Execution.spec.ts, and the end-to-endcode.spec.tsnow assert the failed-session path resolves with a 200 and the log embedded in the response, instead of throwing/400.api/docs/diagrams/to reflect the new resolve-not-throw behaviour and corrected line-number references.Checks
npm run lint:fix).npm test).