fix(mv_getjoblog): surface actual error when job fails before session creation

[sasjs-dev]

Problem

When a Viya JES job fails before a compute session is created (e.g. 403 on POST /compute/contexts/.../sessions), the job response has state=failed and loglocation is missing (SAS missing value '.').

The mv_getjoblog macro only checked for an empty loglocation, not a missing value (.), so it fell through to the existing "validate log path" code — which reported the opaque and unhelpful error:

URI is too short - .

This hid the actual failure reason from the user.

Fix

This patch:

1. Captures jobstate from the JES response (in addition to loglocation)
2. Checks for failed/canceled state OR missing/empty loglocation — including '.' (SAS missing value)
3. Reads the error dataset from the JES response (httpStatusCode, message) when no log is available
4. Reports the real error via mp_abort instead of the opaque "URI is too short" message

Before / After

Before:

MV_GETJOBLOG: jobstate=[failed] loglocation=[.]
NOTE - URI is too short - .
MP_ABORT MSG URI is too short - .

After:

MV_GETJOBLOG: jobstate=[failed] loglocation=[.]
MV_GETJOBLOG: Job failed - error 403 : You are not authorized to submit this request.
NOTE - Job failed, no log available. Error 403: You are not authorized to submit this request.
MP_ABORT MSG Job failed, no log available. Error 403: You are not authorized to submit this request.

Test Plan

• Tested on Viya 2026 (nextviya.emea.sas.com) — the 403 error is now surfaced correctly
• Verify that jobs with valid loglocations still fetch logs normally (no regression)

[4gl-reviewer]

Code Review — 4gl-reviewer

Summary: Good fix — surfaces the actual error from failed Viya compute jobs (e.g. 403 on session creation) instead of the opaque "URI is too short" message. Skills documentation updates are thorough. A few SAS macro issues to address.

Issues:

• viya/mv_getjoblog.sas:186 — call symputx('jobstate',state,'l') assumes the state variable exists in the JSON root dataset. If an older API version or unexpected response omits it, this produces WARNING: Variable state is uninitialized — which the repo's own SAS skill docs (added in this PR) say should be treated as a defect. Consider guarding with a variable-existence check.
• viya/mv_getjoblog.sas:193 — %local jobstate; is declared after call symputx('jobstate',state,'l') already creates the variable. While %local is processed at macro compile time (so it works functionally), it violates the "declare locals at the top" convention this repo follows and is confusing to readers.
• viya/mv_getjoblog.sas:194 — The %put statement executes unconditionally on every call, including successful jobs. Production macros in @sasjs/core typically gate debug output behind a debug flag (e.g. mdebug). Consider removing or guarding it.

Suggestions:

• viya/mv_getjoblog.sas:193 — Move %local jobstate; (and %local err_httpcode err_msg; on line 198) to the top of the macro alongside existing %local declarations, before any data steps that use them.
• viya/mv_getjoblog.sas:195-197 — The outer %if catches jobstate=failed/canceled OR empty loglocation, but the inner %if only checks loglocation. If jobstate=failed but a loglocation exists, execution falls through to normal log fetching — this seems intentional (you can still fetch the log). A one-line comment confirming this would help.

Looks good:

• Error extraction from &libref1..error with stop; is clean and correctly avoids reading past the first error row.
• Defensive %str() wrapping on all macro variable comparisons follows safe SAS macro practice.
• Skills documentation updates (Limitations sections, before/after examples, JS runtime security hardening) are comprehensive and well-structured.

[4gl-reviewer]

Code Review — 4gl-reviewer

Note: This PR mixes code with generated/docs/config files. Reviewing only the code change in viya/mv_getjoblog.sas. The all.sas change is the same macro in the generated concatenation file; .agents/skills/*/SKILL.md, sasjsconfig-schema.json, and skills-lock.json are docs/config — excluded.

Summary: Solid fix that surfaces real Viya job failure reasons (HTTP code + error message) instead of the opaque "URI is too short" message when a job fails before session creation. Logic is correct and well-structured.

Issues:

• [viya/mv_getjoblog.sas:~196] — The outer %if condition includes jobstate=failed or jobstate=canceled, but the immediately nested %if only checks loglocation. When jobstate is failed/canceled and loglocation IS present, the outer block is entered but the inner block's condition is false, so nothing happens — those jobstate checks in the outer condition are effectively dead. The outer %if could be simplified to just %str(&loglocation)= or %str(&loglocation)=. since the inner guard already gates on that.
• [viya/mv_getjoblog.sas:~206] — If &libref1..error table doesn't exist, err_httpcode and err_msg remain empty (declared %local but never assigned), producing an awkward abort message: "Job failed, no log available. Error : ". Consider guarding the Error &err_httpcode: &err_msg suffix with a %length(&err_msg)>0 check, or providing a fallback like "unknown".

Suggestions:

• [viya/mv_getjoblog.sas:~186] — Consider also guarding the case where &libref1..root has zero observations: call symputx('jobstate',state,'l') won't execute, leaving jobstate empty. The current code handles this (empty jobstate falls into the error path), but the abort message would say "Job , no log available..." with a dangling comma. A fallback %let jobstate=unknown; in the %else branch (when state var doesn't exist) would make the message cleaner.

Looks good:

• Correct use of %mf_existvar to defensively check for the state column before referencing it — handles Viya API response variations where state may be absent.
• %sysfunc(exist(&libref1..error)) guard before reading error details is good defensive practice.
• stop; in the error-reading data step prevents unnecessary I/O past the first row.
• err_httpcode/err_msg/jobstate properly declared in %local — no scope leaks.
• Debug %put of jobstate and loglocation at mdebug=1 aids troubleshooting.
• New dependency on mf_existvar correctly documented in the @li header.

[4gl-reviewer]

Code Review — 4gl-reviewer

Summary: Good defensive fix that surfaces actual Viya job error details (HTTP code + message) when a job fails before session creation, instead of the opaque "URI is too short" message. The .agents/skills/ and skills-lock.json changes are docs/config — reviewing only the SAS code in viya/mv_getjoblog.sas.

Issues:

• [viya/mv_getjoblog.sas:196] — If &libref1..error exists but does not contain the httpStatusCode or message columns (e.g. a different Viya API error schema version), the data _null_ step will error. Consider adding an %mf_existvar guard on those columns before the data step, similar to the state variable check above.

Suggestions:

• [viya/mv_getjoblog.sas:200] — The stop; statement is good for short-circuiting after the first error row, but consider noting in a comment that the Viya error response may contain multiple rows and only the first is captured.
• [viya/mv_getjoblog.sas:191] — The comment "If root had zero observations, jobstate was never set" is slightly misleading since the %else branch sets it to 'unknown' — but only if the data step has observations. Consider rewording to clarify that the %let fallback handles the zero-observations case specifically.

Looks good:

• Clean use of %mf_existvar for compile-time column detection — idiomatic SAS macro pattern.
• Defensive fallback chain: state → unknown → error details → generic abort message. Each layer provides progressively more context.
• New %local declarations for jobstate, err_httpcode, err_msg prevent macro variable leakage.
• The two %mp_abort calls (one with error details, one generic) ensure a useful message in all cases.

[4gl-reviewer]

Code Review — 4gl-reviewer

Summary: Good fix — when a Viya job fails before session creation (empty loglocation), the macro now surfaces the actual API error instead of the opaque "URI is too short" message. Logic is sound, defensive coding is thorough, and it follows existing codebase patterns. (Reviewing only the SAS code in viya/mv_getjoblog.sas + all.sas mirror; skill docs and skills-lock.json excluded.)

Looks good:

• Proper %local declarations for all new variables (jobstate, err_httpcode, err_msg, abortmsg)
• %mf_existvar(&libref1..root,state) correctly handles the case where the JSON response lacks a state column — resolved at compile time, conditional call symputx is the right pattern for optional JSON fields
• Zero-observation fallback (%if %str(&jobstate)= %then %let jobstate=unknown;) correctly handles empty root table where the data step body never executes
• %str(&loglocation)=. check catches numeric-missing-to-character conversion (. ) — good defensive edge case handling
• Abort message construction degrades gracefully: works with or without error details, and the err_msg length check prevents an awkward trailing "Error :" when no error table exists
• all.sas mirror updated consistently with the source macro

Suggestions:

• mv_getjoblog.sas:222 — %mp_abort(iftrue=(1=1), ...) is redundant since iftrue defaults to %str(1=1); the existing unconditional abort at line 177 omits it. Harmless, but you could drop it for consistency.
• mv_getjoblog.sas:211-212 — The error table column names httpStatusCode and message are assumed to exist without an %mf_existvar guard (unlike state in root). If the Viya error schema varies, this could produce a "Variable not found" ERROR. Low risk given the well-defined error response structure, but a %mf_existvar check would be consistent with the pattern used for state.

[4gl-reviewer]

Code Review — 4gl-reviewer

Summary: Good defensive enhancement — surfaces the actual error reason (HTTP code + message) when a Viya job fails before session creation, instead of the opaque "URI is too short" message. The code-only change is in viya/mv_getjoblog.sas; the all.sas change is the generated mirror, and the .agents/skills/ + skills-lock.json changes are docs/metadata — excluded from this code review.

Looks good:

• Consolidates new %local vars (jobstate err_httpcode err_msg abortmsg) into the existing statement rather than adding duplicate lines — follows the multi-operand consolidation rule.
• %mf_existvar guard on the state column before referencing it is the right defensive pattern for JSON-parsed datasets where column presence varies.
• stop; after the first observation in the error-table data _null_ step prevents unnecessary row reads — correct.
• Fallback %let jobstate=unknown when root has zero observations ensures the abort message always reads cleanly.
• Checking both %str(&loglocation)= and %str(&loglocation)=. covers both empty-string and numeric-missing cases from JSON parsing — thorough.

Suggestions:

• [viya/mv_getjoblog.sas:~L200] — The error-table data _null_ step references httpStatusCode and message without an mf_existvar guard (unlike the state check above). If the Viya Jobs API ever returns an error table without these columns, the call symputx will ERROR. Consider wrapping in %if %mf_existvar(&libref1..error,httpStatusCode) and %mf_existvar(&libref1..error,message) %then %do for consistency, or document that these columns are guaranteed by the API contract.
• [viya/mv_getjoblog.sas:~L210] — %mp_abort(iftrue=(1=1)...) is the idiomatic force-abort, but a brief comment explaining "1=1 forces unconditional abort" would help future maintainers who don't know mp_abort semantics.