github / github/gh-aw

require-error-code-in-thrown-error: nested identifier aliases inside binary/template messages bypass write-once resolution (fals

Open
#61,284 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)
772

Description

### Rule: `require-error-code-in-thrown-error`

`messageReferencesErrorCode` (eslint-factory/src/rules/require-error-code-in-thrown-error.ts:13-32) recursively inspects `BinaryExpression` operands and `TemplateLiteral` expression slots by checking each `Identifier`'s **name** directly against `ERROR_CODE_PATTERN`. It never routes those nested identifiers through `resolveWriteOnceInitializerChain` (eslint-factory/src/rules/command-initializer-utils.ts), unlike the top-level `Identifier` branch of `auditMessageExpression` (require-error-code-in-thrown-error.ts:85-90), which does.

The result: a write-once local alias for an `ERR_*`/`SAFE_OUTPUT_E0xx` constant is only recognized when it aliases the **entire** message expression. The moment that alias is used as *part of* a binary `+` concatenation or a template-literal interpolation, resolution silently stops at the identifier's bare name and the rule reports a false `missingErrorCode`.

### Repro (minimal, would need to be added as a test case)

```js
const { ERR_API } = require("./error_codes.cjs");
function f() {
const prefix = ERR_API;
throw new Error(prefix + ": failed"); // flagged as missingErrorCode — false positive
}
```

```js
const { ERR_API } = require("./error_codes.cjs");
function f() {
const prefix = ERR_API;
throw new Error(`${prefix}: failed`); // same bug, template-literal form — false positive
}
```

```js
const { ERR_API } = require("./error_codes.cjs");
function helper() {
const prefix = ERR_API;
return prefix + ": failed";
}
function f() {
throw new Error(helper()); // same bug via resolveSimpleLocalCallReturns — false positive
}
```

Trace for the first snippet: `messageArg` is a top-level `BinaryExpression`, so `isAuditableMessageExpression` is true and `auditMessageExpression` calls `messageReferencesErrorCode(node)` first. That function recurses into `left` (`Identifier "prefix"`) and `right` (string literal), testing `"prefix"` against the pattern directly — it never resolves `prefix`'s initializer. Both operands fail the regex, so `messageReferencesErrorCode` returns `false`. Back in `auditMessageExpression`, the node is neither `CallExpression` nor `Identifier` (it's the `BinaryExpression` itself), so execution falls through to the final `return "missingCode"` — even though `prefix` provably resolves to `ERR_API`.

### Why this matters

- All ~128 files that import `error_codes.cjs` are in this rule's blast radius. "Assign the code to a locally-scoped `const` before concatenating" is an ordinary refactor a developer would reach for (e.g. to shorten a long template literal, or to share a prefix across two `throw` sites) — nothing else in the codebase's conventions discourages it.
- Exhaustive search across `actions/setup/js/*.cjs` for `throw new Error( + ...)` and `= fetch(`-style aliasing of error codes found **zero live occurrences** of this exact shape today, so this is a latent/structural gap rather than a currently-misfiring live diagnostic — but it is fully reproducible from the rule's own source and will misfire the instant the idiom appears.

### Suggested fix

In `messageReferencesErrorCode`, when recursing into a `BinaryExpression` operand or a `TemplateLiteral` expression that is itself an `Identifier`, resolve it via `resolveWriteOnceInitializerChain(expr, sourceCode)` before testing the name (mirroring what the top-level `Identifier` branch of `auditMessageExpression` already does). Note `messageReferencesErrorCode` doesn't currently receive `sourceCode`/`context`, so this requires threading it through (or moving the resolution into a wrapper in `auditMessageExpression` that pre-resolves `BinaryExpression`/`TemplateLiteral` operands before delegating to the pattern check).

### Acceptance criteria

- [ ] The three repro snippets above are added as `valid` cases in `require-error-code-in-thrown-error.test.ts` and pass.
- [ ] A case where the aliased operand does **not** resolve to a code (e.g. `const prefix = "no code"; throw new Error(prefix + ": failed");`) remains correctly flagged as `missingErrorCode`, to confirm the fix doesn't just suppress the whole branch.

> [!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/35059244673) · claude · agent · 318.3 AIC · ⌖ 7.46 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 22, 2026, 9:36 PM UTC-08:00

Contributor guide

Open the contributing guide

Research direction

Start in eslint-factory/src/rules/require-error-code-in-thrown-error.ts, especially messageReferencesErrorCode and auditMessageExpression, then read resolveWriteOnceInitializerChain in eslint-factory/src/rules/command-initializer-utils.ts. Run the cases in require-error-code-in-thrown-error.test.ts; done means the three aliased forms pass as valid while a non-code alias remains reported as missingErrorCode.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
testing-qa, tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.