Skip to content

[Refactor]: Replace cast in causes/index.ts with a structural guard #74

Description

@martyy-code

Current State

In packages/errors/src/causes/index.ts:30, the function-scope variable instance is used as the local name for what is actually a narrowed ErrorInstance:

const causes = (error: unknown): Error[] => {
  if (error == null) {
    return [];
  }

  // Get the causes array from the error
  const instance = error as ErrorInstance;

Rule 0005 (Named Algorithms and Independent Data Structures) treats project-internal diminutives — names that train the reader to translate every line — as a tax. instance is a JavaScript keyword-adjacent word that does not name a concept; in this function it is the receiver of an unsafe cast. The function is 13 lines long; the name carries no domain meaning.

Located in:

  • packages/errors/src/causes/index.ts:30, 36, 38

Problems with current implementation:

  • instance is not a domain concept; it is a JavaScript-shape term. The function deals with errors, not instances.
  • The reader has to remember that instance means "the error we just cast" rather than "an instance of something" — the cast is the only thing giving the name meaning.
  • The neighbouring parameter is named error; instance next to it is redundant.

Proposed State

After refactoring, the variable is renamed to errorInstance (full form, no truncation) and the function uses an early-return that makes the narrowing explicit:

const causes = (error: unknown): Error[] => {
  if (error == null) {
    return [];
  }
  if (!('causes' in error) || !Array.isArray(error.causes)) {
    return [];
  }
  return error.causes;
}

The shape of the cast is preserved (as ErrorInstance was unsound — the new code replaces it with a structural guard, which is the rule 0004 / 0008 cooperative fix).

Expected improvements:

  • Rule 0005 compliance: no project-internal diminutives.
  • Rule 0004 compliance: the runtime guard names its scenario ("causes is an array") instead of relying on a cast.
  • Rule 0008 compliance: the cast at the call site is removed.
  • The function reads top-down (rule 0007): each branch is a name, no cast survives the null guard.

Motivation

This refactoring is needed because:

  • The variable is a diminutive that a reader has to mentally expand on every read.
  • The function is exported as part of the public API (causes); the name carries weight for consumers who skim the source.
  • Combining the rename with the structural-guard refactor kills three birds (0005, 0004, 0008) in one pass.

Triggers for this work:

  • Technical debt accumulation
  • Maintainability concerns

Risks

Potential risks:

  • Risk 1: A consumer relies on causes() returning [] for inputs that are not null but also don't have a causes array. — Mitigation: the current code does instance as ErrorInstance then checks Array.isArray(instance.causes). If the cast succeeds (the input has a causes property of any shape), the function returns that shape. The new code preserves the same behaviour through a structural check.
  • Risk 2: The error.causes property does not exist on native Error instances. — Mitigation: the early return if (!('causes' in error) || !Array.isArray(error.causes)) return []; covers this; native errors return [] as before.

Migration Plan

Migration approach:

  1. Replace the cast + Array.isArray check with a single structural guard.
  2. Rename instance to errorInstance (or remove the local entirely by inlining the structural check, which the early-return shape makes natural).
  3. Update tests if they assert on the cast's runtime behaviour.

Rollback plan: revert the PR. The change is local.

Backward Compatibility

  • This refactoring maintains full backward compatibility

Scope

Files/Folders affected:

  • packages/errors/src/causes/index.ts (the function body)
  • packages/errors/tests/ (if any test exercises the cast path)

Component(s) Affected

  • Multiple Components

Note: the component_affected dropdown is calibrated for a web template project. The actual affected component is packages/errors.

Priority

  • p0: Critical - Blocking major work or causing bugs
  • p1: High - Important, should do soon
  • p2: Medium - Normal priority
  • p3: Low - Nice to have

Estimated Effort

  • effort: xs - Few minutes

Test Coverage Requirements

  • Existing tests cover this code area (will update)

Testing Approach

Verification steps:

  1. pnpm --filter @deessejs/errors test:run
  2. pnpm --filter @deessejs/errors type-check

Related Issues / Pull Requests

  • Related audit: P0 feat: implement .from() method for exception chaining #4 in the internal audit of packages/errors/src/ against rules 0001-0016, August 2026.
  • Related rule: docs/engineering/architecture/rules/0005-named-algorithms-and-independent-data-structures.md.
  • Related rule: docs/engineering/architecture/rules/0004-no-speculative-defences.md.
  • Related rule: docs/engineering/architecture/rules/0008-no-chained-type-assertions.md.

Relevant Documentation

  • Architecture doc: docs/engineering/architecture/rules/0005-named-algorithms-and-independent-data-structures.md

Pre-Submission Checklist

  • I have searched existing issues for related refactoring requests
  • Risks and migration plan are documented
  • Test coverage approach is defined
  • I understand this issue will be labeled according to the project taxonomy
  • This is NOT a security vulnerability (see security note above)

Metadata

Metadata

Assignees

No one assigned

    Labels

    p1: highRequired for next releasetype: refactorRefactoring / code restructuring

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions