diff --git a/api/src/controllers/internal/Execution.ts b/api/src/controllers/internal/Execution.ts index ad11706..7d5f978 100644 --- a/api/src/controllers/internal/Execution.ts +++ b/api/src/controllers/internal/Execution.ts @@ -144,7 +144,16 @@ export class ExecutionController { : '' // it should be deleted by scheduleSessionDestroy - session.state = SessionState.completed + // + // 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 = [] diff --git a/api/src/controllers/internal/spec/Execution.spec.ts b/api/src/controllers/internal/spec/Execution.spec.ts index 3640d88..e377d12 100644 --- a/api/src/controllers/internal/spec/Execution.spec.ts +++ b/api/src/controllers/internal/spec/Execution.spec.ts @@ -14,7 +14,7 @@ const preProgramVariables: PreProgramVars = { httpHeaders: [] } -describe('ExecutionController.executeProgram (SAS failure path)', () => { +describe('ExecutionController.executeProgram', () => { let session: Session beforeEach(() => { @@ -33,33 +33,85 @@ describe('ExecutionController.executeProgram (SAS failure path)', () => { await deleteFolder(session.path) }) - it('throws a SessionExecutionError carrying the complete log when the session fails', async () => { - const logPath = path.join(session.path, 'log.log') - const logContent = - 'NOTE: SAS session\nERROR: SAS session terminated. See log for details.\n' + // 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. + describe('JS/PY/R failure 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' + }) - await createFile(logPath, logContent) + const controller = new ExecutionController() - jest - .spyOn(ProcessProgramModule, 'processProgram') - .mockImplementation(async () => { - throw new Error('ERROR: SAS session terminated. See log for details.') + await controller.executeProgram({ + program: 'throw new Error("boom")', + preProgramVariables, + vars: {}, + session, + runTime: RunTimeType.JS }) - const controller = new ExecutionController() - - const resultPromise = controller.executeProgram({ - program: '%abort;', - preProgramVariables, - vars: {}, - session, - runTime: RunTimeType.SAS + expect(session.state).toBe(SessionState.failed) }) - await expect(resultPromise).rejects.toBeInstanceOf(SessionExecutionError) - await expect(resultPromise).rejects.toMatchObject({ - log: logContent, - message: expect.stringContaining('SAS session terminated') + 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) + }) + }) + + describe('SAS failure path', () => { + it('throws a SessionExecutionError carrying the complete log when the session fails', async () => { + const logPath = path.join(session.path, 'log.log') + const logContent = + 'NOTE: SAS session\nERROR: SAS session terminated. See log for details.\n' + + await createFile(logPath, logContent) + + jest + .spyOn(ProcessProgramModule, 'processProgram') + .mockImplementation(async () => { + throw new Error('ERROR: SAS session terminated. See log for details.') + }) + + const controller = new ExecutionController() + + const resultPromise = controller.executeProgram({ + program: '%abort;', + preProgramVariables, + vars: {}, + session, + runTime: RunTimeType.SAS + }) + + await expect(resultPromise).rejects.toBeInstanceOf(SessionExecutionError) + await expect(resultPromise).rejects.toMatchObject({ + log: logContent, + message: expect.stringContaining('SAS session terminated') + }) }) }) }) diff --git a/api/src/routes/api/spec/drive.spec.ts b/api/src/routes/api/spec/drive.spec.ts index c4c28ce..c6bfd9d 100644 --- a/api/src/routes/api/spec/drive.spec.ts +++ b/api/src/routes/api/spec/drive.spec.ts @@ -28,6 +28,15 @@ jest .spyOn(fileUtilModules, 'getUploadsFolder') .mockImplementation(() => path.join(tmpFolder, 'uploads')) +// getFilesFolder() resolves via getSasjsDriveFolder()/process.driveLoc, a +// separate root from getSasjsRootFolder()/process.sasjsRoot above - without +// this, every test in this file that creates content under the drive (e.g. +// 'level1', 'my/path/...') writes into the real, shared sasjs_root/drive +// instead of this run's isolated tmpFolder, leaking state into later runs. +jest + .spyOn(fileUtilModules, 'getFilesFolder') + .mockImplementation(() => path.join(tmpFolder, 'drive', 'files')) + import appPromise from '../../../app' import { UserController,