no-err-stack-then-string-fallback misses getErrorMessage() fallback and combined instanceof+.stack test shape (8 live escapes)
- Dominant language
- Go
- Stars
- 5.1k
- Forks
- 541
- Avg merge
- 5h 48m
- Merged PRs (30d)
- 773
Description
### Summary
`no-err-stack-then-string-fallback` only recognizes two exact ternary shapes:
- `errVar && errVar.stack ? errVar.stack : String(errVar)`
- `errVar instanceof Error ? errVar.stack : String(errVar)`
Both the truthy-test matcher and the alternate-branch matcher are too narrow, and the result is that **every single live call site of this idiom in `actions/setup/js/**` currently escapes detection**.
### Gap 1 — alternate branch hard-codes `String(errVar)`, misses `getErrorMessage(errVar)`
`isStringErr()` in `eslint-factory/src/rules/no-err-stack-then-string-fallback.ts` only matches `String(errVar)` as the alternate. It does not recognize `getErrorMessage(errVar)` — even though that's literally the rule's own suggested fix. Code that already migrated the *fallback* arm to `getErrorMessage` but left the *truthy* arm printing a raw stack trace is invisible to the rule:
```js
core.setFailed(err && err.stack ? err.stack : getErrorMessage(err));
```
Live occurrences (non-test `.cjs`):
- `actions/setup/js/apply_samples.cjs:720` — inside `core.setFailed(...)`, so a raw stack trace becomes the GitHub Actions failure annotation shown to users.
- `actions/setup/js/merge_remote_agent_github_folder.cjs:459` — same `core.setFailed(...)` pattern.
- `actions/setup/js/parse_mcp_gateway_log.cjs:1341` — `console.error(err && err.stack ? err.stack : getErrorMessage(err))`.
### Gap 2 — combined `instanceof Error && .stack` test shape is entirely unhandled
The `ConditionalExpression` handler resolves `errVar` from `test.left`, expecting `test.left` to be a bare `Identifier`. The dominant idiom in this codebase, however, combines both checks in one `LogicalExpression`:
```js
err instanceof Error && err.stack ? err.stack : getErrorMessage(err)
```
Here `test.left` is `err instanceof Error` (a `BinaryExpression`), not an `Identifier`, so `isErrAndErrStack`/`isErrInstanceofError` never even get a chance to match — the handler returns immediately. This exact shape is the **most common** stack-fallback idiom in the corpus:
- `actions/setup/js/copilot_sdk_driver.cjs:141`
- `actions/setup/js/pi_agent_core_driver.cjs:404`
- `actions/setup/js/parse_mcp_scripts_logs.cjs:416`
- `actions/setup/js/detect_agent_errors.cjs:616`
- `actions/setup/js/create_issue.cjs:1385`
### Impact
Combined, **8 live, non-test call sites** hit the exact anti-pattern this rule exists to catch (dumping a noisy stack trace where a clean `getErrorMessage()` result belongs), and none of them are flagged. Two of them (`apply_samples.cjs`, `merge_remote_agent_github_folder.cjs`) feed directly into `core.setFailed()`, so the noisy stack trace is what a user sees as the step failure reason.
Confirmed neither shape is covered by the existing test suite (`no-err-stack-then-string-fallback.test.ts`) — no test uses `getErrorMessage` as the alternate, and none use the combined `instanceof && .stack` test.
### Suggested fix
1. Extend `isStringErr()` (or add a sibling `isGetErrorMessageErr()`) to also accept `getErrorMessage(errVar)` as a valid "safe fallback" alternate — reporting `preferGetErrorMessage` still makes sense since the *truthy* branch is the actual problem, and the autofix should collapse to `getErrorMessage(errVar)` either way.
2. In the `ConditionalExpression` handler, when `test` is a `LogicalExpression` with `&&`, also accept `test.left` being `errVar instanceof Error` (delegate to `isErrInstanceofError`) in addition to the current bare-identifier case, so `errVar instanceof Error && errVar.stack ? ... : ...` resolves `errVar` correctly.
### Acceptance criteria
- [ ] `errVar && errVar.stack ? errVar.stack : getErrorMessage(errVar)` is flagged and autofixes to `getErrorMessage(errVar)`.
- [ ] `errVar instanceof Error && errVar.stack ? errVar.stack : getErrorMessage(errVar)` is flagged and autofixes to `getErrorMessage(errVar)`.
- [ ] `errVar instanceof Error && errVar.stack ? errVar.stack : String(errVar)` (same combined test, `String` fallback) is also flagged.
- [ ] New test cases added covering both gaps above, using the exact live shapes from `apply_samples.cjs:720` and `copilot_sdk_driver.cjs:141`.
- [ ] Existing valid/invalid tests in `no-err-stack-then-string-fallback.test.ts` continue to pass.
_Scope: `eslint-factory/src/rules/no-err-stack-then-string-fallback.ts` (+ its `.test.ts`); target corpus `actions/setup/js/**`._
> [!WARNING]
>
> Firewall blocked 1 domain
>
> The following domain was blocked by the firewall during workflow execution:
>
> - `api.anthropic.com`
>
> To allow these domains, add them to the `network.allowed` list in your workflow frontmatter:
>
> ```yaml
> network:
> allowed:
> - defaults
> - "api.anthropic.com"
> ```
>
> See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information.
>
>
> Generated by [🤖 ESLint Refiner](https://github.com/github/gh-aw/actions/runs/34565635777) · claude · agent · 280.2 AIC · ⌖ 7.75 AIC · ⊞ 5.8K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw+is%3Aissue+%22gh-aw-workflow-call-id%3A+github%2Fgh-aw%2Feslint-refiner%22&type=issues)
> - [x] expires on Sep 17, 2026, 9:53 PM UTC-08:00
Contributor guide
Research direction
Read eslint-factory/src/rules/no-err-stack-then-string-fallback.ts and its tests in no-err-stack-then-string-fallback.test.ts, then run the existing rule tests. Add cases matching the live shapes at actions/setup/js/apply_samples.cjs:720 and copilot_sdk_driver.cjs:141, including both fallback forms and autofix behavior. Done means all acceptance cases pass without regressing existing valid and invalid tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- eslint, github-actions, javascript, typescript
- Domain
- testing-qa, tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 84/100