fix(job-execution): return Internal Server Error code in case of Stored Process Error

[krishna-acondy]

Issue

sasjs/cli#1063

Intent

Return the correct error code when there is a SAS Stored Process Error.

Implementation

Changed 404 to 500.

Checks

No PR (that involves a non-trivial code change) should be merged, unless all items below are confirmed! If an urgent fix is needed - use a tar file.

• All sasjs-cli unit tests are passing (npm test).
• All sasjs-tests are passing (instructions available here).
• Data Controller builds and is functional on both SAS 9 and Viya

[github-actions]

Coverage report

Total coverage

Status	Category	Percentage	Covered / Total
🔴	Statements	58.03%	1605/2766
🔴	Branches	39.16%	551/1407
🔴	Functions	45.63%	230/504
🟡	Lines	67.03%	2568/3831

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Report generated by 🧪jest coverage report action from 596c1de

[4gl-reviewer]

Hermes Agent Code Review

Verdict: APPROVE (with nits) — the change is correct, minimal, and low-risk. A small documentation/test follow-up is suggested but does not block merge.

Summary

This PR changes the JobExecutionError error code returned by parseError for the "Stored Process Error / This request completed with errors." branch from 404 to 500.

A 404 semantically means Not Found, which is incorrect for a Stored Process that was located and executed but returned an error during execution. 500 (Internal Server Error) is the right status for a server-side execution failure, so this is a correct fix.

Correctness ✅

• The change is a single literal swap (404 → 500) at src/request/RequestClient.ts:630, matching the surrounding pattern. The adjacent "stored process not found" branch (line 616) correctly keeps 404, so the two cases are now semantically distinct and consistent.
• JobExecutionError (src/types/errors/JobExecutionError.ts) accepts errorCode: number, so 500 is type-compatible. The value is stored on this.errorCode and surfaced via the error message string Error Code ${errorCode}: ${errorMessage}.
• Downstream handlers in WebJobExecutor, FileUploader, and JesJobExecutor branch on e instanceof JobExecutionError, not on the numeric errorCode, so there is no change to control flow, retry behaviour, or error propagation. Confirmed no code in src/job-execution/ or src/request/ switches on errorCode === 404 or similar.
• appendRequest does not consume errorCode; it only records logFile/serviceLink. So the value change has no side effects on request logging.

Security ✅

• No new inputs, no network surface change, no secrets handling. The error code is derived from a static literal, not from untrusted response data (unlike the responseJson.errorCode branch which already forwards server-supplied codes). No injection or information-disclosure concern.

Tests ⚠️

• src/test/RequestClient.spec.ts does not cover parseError at all — neither the 404 "not found" branch nor this 500 "Stored Process Error" branch. There are no tests asserting on errorCode for any parseError path.
• Suggestion (non-blocking): add a unit test for parseError covering the "Stored Process Error" HTML payload, asserting that a JobExecutionError with errorCode === 500 is returned. This would lock in the new behaviour and prevent regression. Since parseError is not exported, the test could drive it through the public parseWeboutResponse/request error path or via a small export.

Style ✅

• Change is consistent with the file's existing style. No formatting concerns (prettier would not flag a numeric literal change).

Performance ✅

• No impact — identical runtime cost.

Notes

• The PR is marked Draft and the PR body checkboxes (unit tests / sasjs-tests / Data Controller) are all unchecked. The author should confirm at least npm test passes before flipping to ready-for-review, though the one-line nature makes a regression unlikely.
• The linked issue (sasjs/cli#1063) motivates returning the correct status code to callers; this change satisfies that intent for the adapter side.

Reviewed by Hermes Agent (GitHub App)

[4gl-reviewer]

Correct fix — 500 is the right code for a Stored Process that ran but errored, while the 404 on line 616 correctly remains for the genuinely-not-found case.

Nit: there are currently no unit tests covering parseError (see src/test/RequestClient.spec.ts). Consider adding a test that feeds a "Stored Process Error ... This request completed with errors. ... <h2>SAS Log</h2><pre>...</pre>" payload and asserts errorCode === 500 so this behaviour is locked in.