Skip to content

eslint-factory: no-json-stringify-error suggestion is message-only — at the sole live site it duplicates an adjacent getErrorMes [Content truncated due to length] #43718

Description

@github-actions

Rule

eslint-factory/src/rules/no-json-stringify-error.ts (new 10th rule; first review this run).

The rule correctly flags JSON.stringify(caughtErrorVar) (Error props are non-enumerable → {}). It offers a single suggestion: replace the whole call with getErrorMessage(errorVar).

Grounding (corpus: actions/setup/js/**/*.cjs)

The rule has exactly one live true-positive site and it catches it:

// actions/setup/js/expired_entity_cleanup_helpers.cjs:132-140
} catch (error) {
  core.error(`✗ Failed to close ... : ${getErrorMessage(error)}`);          // line 133
  core.error(`  Error details: ${JSON.stringify(error, null, 2)}`);          // line 134  ← flagged (produces {})
  failed.push({ ..., error: getErrorMessage(error) });                       // line 139
}

Applying the rule's getErrorMessage(error) suggestion to line 134 yields:

core.error(`  Error details: ${getErrorMessage(error)}`);

That removes the {} bug, but the result is redundant — line 133 already logs exactly getErrorMessage(error) — and it discards the developer's evident intent: the " Error details:" label plus JSON.stringify(..., null, 2) pretty-printing was meant to surface fuller detail than the message alone.

getErrorMessage (actions/setup/js/error_helpers.cjs:26-44) returns only error.message — no stack, no other properties. So the single suggestion always narrows detail to the message, which is the opposite of what a JSON.stringify(err, null, 2) call site usually wants.

Secondary concern: the suggestion inserts getErrorMessage(...) unconditionally. It happens to be imported at this one site, but on any file lacking the import, applying the suggestion introduces a ReferenceError. The message text asks the user to "ensure getErrorMessage is imported" but the rule performs no scope check.

Ask / acceptance criteria

  1. Add a second suggestion that preserves detail, e.g. replace with `${getErrorMessage(errorVar)}\n${errorVar.stack ?? ""}` or a guarded property dump — so users who wanted null, 2-style detail retain the stack. Keep the message-only suggestion as the concise option.
  2. Make the getErrorMessage suggestion scope-aware: when getErrorMessage is not resolvable in the file's scope, fall back to suggesting String(errorVar) (needs no import) instead of an unresolved identifier.
  3. Tests: add invalid cases asserting (a) the detail-preserving suggestion output, and (b) the String(errorVar) fallback on a file with no getErrorMessage import.

Not asking for

No change to the primary detection (Identifier-only, catch/.catch()-scoped) — it is precise and produced 0 false positives on the corpus (error.data / error.request member accesses in github_api_helpers.cjs:56-57 are plain objects and correctly not flagged).

Generated by 🤖 ESLint Refiner · 261.4 AIC · ⌖ 12.3 AIC · ⊞ 4.7K ·

  • expires on Jul 12, 2026, 10:53 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions