github / github/gh-aw

no-err-stack-then-string-fallback misses getErrorMessage() fallback and combined instanceof+.stack test shape (8 live escapes)

Open
#60,196 0 comments 0 reactions 0 assignees View on GitHub
cookie eslint eslint-factory
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.