mirror of
https://github.com/sasjs/server.git
synced 2026-07-24 05:32:15 +00:00
fix(api): return 200 with embedded log on SAS session failure instead 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.
This commit is contained in:
@@ -114,8 +114,7 @@ const executeCode = async (
|
||||
code: 400,
|
||||
status: 'failure',
|
||||
message: 'Job execution failed.',
|
||||
error: typeof err === 'object' ? err.toString() : err,
|
||||
log: err?.log
|
||||
error: typeof err === 'object' ? err.toString() : err
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,24 +15,6 @@ export interface ExecutionVars {
|
||||
[key: string]: string | number | undefined
|
||||
}
|
||||
|
||||
// Thrown when the session itself fails (e.g. SAS exits abnormally via
|
||||
// %abort;). Carries the complete log - by the time this is thrown, the
|
||||
// session's process has already exited, so the log file it wrote is final,
|
||||
// not a partial/truncated snapshot.
|
||||
export class SessionExecutionError extends Error {
|
||||
constructor(
|
||||
message: string,
|
||||
public log?: string
|
||||
) {
|
||||
super(message)
|
||||
|
||||
// required for `instanceof` to work when compiling to ES5, since the
|
||||
// default __extends helper does not preserve the prototype chain for
|
||||
// classes extending built-ins like Error
|
||||
Object.setPrototypeOf(this, SessionExecutionError.prototype)
|
||||
}
|
||||
}
|
||||
|
||||
export interface ExecuteReturnRaw {
|
||||
httpHeaders: HTTPHeaders
|
||||
result: string | Buffer
|
||||
@@ -106,24 +88,23 @@ export class ExecutionController {
|
||||
preProgramVariables?.httpHeaders.join('\n') ?? ''
|
||||
)
|
||||
|
||||
try {
|
||||
await processProgram(
|
||||
program,
|
||||
preProgramVariables,
|
||||
vars,
|
||||
session,
|
||||
weboutPath,
|
||||
headersPath,
|
||||
tokenFile,
|
||||
runTime,
|
||||
logPath,
|
||||
otherArgs
|
||||
)
|
||||
} catch (err: any) {
|
||||
const log = (await fileExists(logPath)) ? await readFile(logPath) : ''
|
||||
|
||||
throw new SessionExecutionError(err.message, log)
|
||||
}
|
||||
// A failed session (e.g. SAS via %abort;, or a non-zero JS/PY/R exit)
|
||||
// is a normal outcome of running arbitrary user code, not a
|
||||
// request-shape/server problem - processProgram resolves rather than
|
||||
// throwing in that case, and the session.failureReason check below
|
||||
// folds the log into the same result shape a successful run returns.
|
||||
await processProgram(
|
||||
program,
|
||||
preProgramVariables,
|
||||
vars,
|
||||
session,
|
||||
weboutPath,
|
||||
headersPath,
|
||||
tokenFile,
|
||||
runTime,
|
||||
logPath,
|
||||
otherArgs
|
||||
)
|
||||
|
||||
const log = (await fileExists(logPath)) ? await readFile(logPath) : ''
|
||||
const headersContent = (await fileExists(headersPath))
|
||||
|
||||
@@ -48,12 +48,17 @@ export const processProgram = async (
|
||||
await createFile(codePath + '.bkp', program)
|
||||
await moveFile(codePath + '.bkp', codePath)
|
||||
|
||||
// we now need to poll the session status
|
||||
while (session.state !== SessionState.completed) {
|
||||
if (session.state === SessionState.failed) {
|
||||
throw new Error(session.failureReason || 'SAS session failed')
|
||||
}
|
||||
|
||||
// we now need to poll the session status. A failed session (e.g. from
|
||||
// %abort;) is not a request-shape/server problem - it's a normal
|
||||
// outcome of running arbitrary user code, same as a SAS ERROR: in the
|
||||
// log without %abort;. So we just stop polling rather than throwing;
|
||||
// Execution.ts already knows how to turn session.failureReason into a
|
||||
// 200 response with the log embedded, matching how JS/PY/R (the else
|
||||
// branch below) has always handled a failed session.
|
||||
while (
|
||||
session.state !== SessionState.completed &&
|
||||
session.state !== SessionState.failed
|
||||
) {
|
||||
await delay(50)
|
||||
}
|
||||
} else {
|
||||
|
||||
@@ -2,7 +2,7 @@ import path from 'path'
|
||||
import os from 'os'
|
||||
import { createFile, deleteFolder, generateTimestamp } from '@sasjs/utils'
|
||||
import * as ProcessProgramModule from '../processProgram'
|
||||
import { ExecutionController, SessionExecutionError } from '../Execution'
|
||||
import { ExecutionController } from '../Execution'
|
||||
import { Session, SessionState, PreProgramVars } from '../../../types'
|
||||
import { RunTimeType } from '../../../utils'
|
||||
|
||||
@@ -83,8 +83,15 @@ describe('ExecutionController.executeProgram', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// A failed SAS session (e.g. from %abort;) is a normal outcome of
|
||||
// running arbitrary user code, not a request-shape/server problem - the
|
||||
// same way a plain SAS ERROR: in the log (without %abort;) already
|
||||
// returns 200 with the log embedded. This mirrors the JS/PY/R failure
|
||||
// path above: resolve normally, don't throw, and let the existing
|
||||
// session.failureReason check below produce the same result shape as a
|
||||
// successful run.
|
||||
describe('SAS failure path', () => {
|
||||
it('throws a SessionExecutionError carrying the complete log when the session fails', async () => {
|
||||
it('returns the complete log embedded in a normal result instead of throwing, 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'
|
||||
@@ -94,12 +101,17 @@ describe('ExecutionController.executeProgram', () => {
|
||||
jest
|
||||
.spyOn(ProcessProgramModule, 'processProgram')
|
||||
.mockImplementation(async () => {
|
||||
throw new Error('ERROR: SAS session terminated. See log for details.')
|
||||
// mirrors the real SAS branch after the fix: sets
|
||||
// state/failureReason and resolves, exactly like the JS/PY/R
|
||||
// branch already does - it does not throw
|
||||
session.state = SessionState.failed
|
||||
session.failureReason =
|
||||
'ERROR: SAS session terminated. See log for details.'
|
||||
})
|
||||
|
||||
const controller = new ExecutionController()
|
||||
|
||||
const resultPromise = controller.executeProgram({
|
||||
const { result } = await controller.executeProgram({
|
||||
program: '%abort;',
|
||||
preProgramVariables,
|
||||
vars: {},
|
||||
@@ -107,11 +119,8 @@ describe('ExecutionController.executeProgram', () => {
|
||||
runTime: RunTimeType.SAS
|
||||
})
|
||||
|
||||
await expect(resultPromise).rejects.toBeInstanceOf(SessionExecutionError)
|
||||
await expect(resultPromise).rejects.toMatchObject({
|
||||
log: logContent,
|
||||
message: expect.stringContaining('SAS session terminated')
|
||||
})
|
||||
expect(session.state).toBe(SessionState.failed)
|
||||
expect(result).toEqual(expect.stringContaining(logContent))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -65,7 +65,13 @@ describe('processProgram (SAS runtime)', () => {
|
||||
await deleteFolder(session.path)
|
||||
})
|
||||
|
||||
it('rejects instead of hanging when the session fails (e.g. %abort;)', async () => {
|
||||
it('resolves instead of hanging when the session fails (e.g. %abort;)', async () => {
|
||||
// mirrors the JS/PY/R branch below: a session failure is a normal
|
||||
// outcome of running arbitrary user code (like a SAS ERROR: in the
|
||||
// log without %abort;), not a server-side/request-shape problem - so
|
||||
// processProgram must not throw here, just stop polling. Execution.ts
|
||||
// is responsible for turning session.failureReason into a 200 response
|
||||
// with the log embedded, the same way it already does for JS/PY/R.
|
||||
setTimeout(() => {
|
||||
session.state = SessionState.failed
|
||||
session.failureReason =
|
||||
@@ -84,7 +90,7 @@ describe('processProgram (SAS runtime)', () => {
|
||||
RunTimeType.SAS,
|
||||
logPath
|
||||
)
|
||||
).rejects.toThrow(/SAS session terminated/)
|
||||
).resolves.toBeUndefined()
|
||||
}, 3000)
|
||||
|
||||
it('resolves without throwing when the session completes normally', async () => {
|
||||
|
||||
@@ -166,8 +166,7 @@ const execute = async (
|
||||
code: 400,
|
||||
status: 'failure',
|
||||
message: 'Job execution failed.',
|
||||
error: typeof err === 'object' ? err.toString() : err,
|
||||
log: err?.log
|
||||
error: typeof err === 'object' ? err.toString() : err
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user