1
0
mirror of https://github.com/sasjs/server.git synced 2026-07-23 21:25:29 +00:00

fix(api): stop overwriting a failed JS/PY/R session back to completed

For JS/PY/R, processProgram sets session.state = failed (with
failureReason) itself when the interpreter process exits non-zero,
without throwing. ExecutionController.executeProgram then
unconditionally set session.state = completed right after
processProgram returned, silently overwriting that - so anything
downstream inspecting session.state (e.g. scheduleSessionDestroy's
expiresAfterMins branch) would see a crashed session mis-reported as
successful.

Guard the assignment so a failed state is never overwritten. No
effect on SAS, which sets state independently via its own spawned
process lifecycle.

Added Execution.spec.ts covering both the failure case (state stays
failed) and the success case (state still becomes completed), to
guard against regressing in either direction.
This commit is contained in:
YuryShkoda
2026-07-14 11:29:30 +03:00
parent 8b2a6155a9
commit 4858245372
2 changed files with 94 additions and 1 deletions
@@ -120,7 +120,16 @@ export class ExecutionController {
: ''
// it should be deleted by scheduleSessionDestroy
//
// Guarded: for JS/PY/R, processProgram sets state to `failed` itself
// (without throwing) when the interpreter process exits non-zero - if
// we unconditionally set `completed` here we'd silently overwrite that,
// and anything downstream inspecting session.state (e.g.
// scheduleSessionDestroy's expiresAfterMins branch in Session.ts) would
// see a crashed session mis-reported as successful.
if ((session.state as SessionState) !== SessionState.failed) {
session.state = SessionState.completed
}
const resultParts = []
@@ -0,0 +1,84 @@
import path from 'path'
import os from 'os'
import { deleteFolder, generateTimestamp } from '@sasjs/utils'
import * as ProcessProgramModule from '../processProgram'
import { ExecutionController } from '../Execution'
import { Session, SessionState, PreProgramVars } from '../../../types'
import { RunTimeType } from '../../../utils'
// Regression coverage: for JS/PY/R, processProgram sets session.state to
// `failed` itself (without throwing) on a non-zero interpreter exit.
// ExecutionController.executeProgram used to unconditionally overwrite
// that back to `completed` right after processProgram returned, silently
// losing the failure for anything downstream that inspects session.state.
const preProgramVariables: PreProgramVars = {
username: 'testUser',
userId: 1,
displayName: 'Test User',
serverUrl: 'http://localhost:5000',
httpHeaders: []
}
describe('ExecutionController.executeProgram (JS/PY/R failure path)', () => {
let session: Session
beforeEach(() => {
const sessionId = `test-session-${generateTimestamp()}`
session = {
id: sessionId,
state: SessionState.pending,
creationTimeStamp: '0',
deathTimeStamp: '0',
path: path.join(os.tmpdir(), sessionId)
}
})
afterEach(async () => {
jest.restoreAllMocks()
await deleteFolder(session.path)
})
it('does not overwrite a failed session state back to completed', async () => {
// mirrors processProgram's real JS/PY/R branch on a non-zero exit:
// it sets state/failureReason itself and resolves - it does not throw
jest
.spyOn(ProcessProgramModule, 'processProgram')
.mockImplementation(async () => {
session.state = SessionState.failed
session.failureReason = 'Error: process exited with code 1'
})
const controller = new ExecutionController()
await controller.executeProgram({
program: 'throw new Error("boom")',
preProgramVariables,
vars: {},
session,
runTime: RunTimeType.JS
})
expect(session.state).toBe(SessionState.failed)
})
it('still marks a genuinely successful session as completed', async () => {
jest
.spyOn(ProcessProgramModule, 'processProgram')
.mockImplementation(async () => {
session.state = SessionState.completed
})
const controller = new ExecutionController()
await controller.executeProgram({
program: 'console.log("hello")',
preProgramVariables,
vars: {},
session,
runTime: RunTimeType.JS
})
expect(session.state).toBe(SessionState.completed)
})
})