Skip to content

[Refactor]: Top-down composition audit across packages/errors public surface #81

Description

@martyy-code

Current State

Several functions in packages/errors/src/ do not read top-down (rule 0007). The smell is the canonical one the rule was written to catch: the first line is the function name, and the body is 30-60 lines of mechanism the reader has to parse to recover the intent.

Examples:

  • packages/errors/src/error/error.ts:87-134 — the ErrorFactoryInstance inner function is 48 lines. The first line says "create an instance"; the body is six things: format message, capture stack, build instance, attach from, attach addNote, mark factory. The reader has to parse all six to know what the function does.
  • packages/errors/src/is/index.ts:56-116 — the is() function is 60 lines. The first line says "discriminate an error against a factory"; the body is four concerns: null check, native instanceof check, factory-symbol check with DFS walk, default. The DFS walk alone is 30 of the 60 lines.
  • packages/errors/src/causes/index.ts:30-43 — the causes() function is 13 lines. Less severe, but the cast + Array.isArray pattern obscures the intent: the function is "return the cause chain, or an empty array if there is none".

The P0 issues #71, #72, #73, #77, and the new #9 (extracted inheritance walk) touch parts of this problem. This issue is the umbrella: the top-down reading discipline has not been audited across the package's public surface.

Located in:

  • packages/errors/src/error/error.ts:87-134
  • packages/errors/src/is/index.ts:56-116
  • packages/errors/src/causes/index.ts:30-43
  • (Other functions in the package should be reviewed for the same pattern)

Proposed State

After this task, every public function in packages/errors/src/ reads top-down. Concretely:

Expected improvements:

Motivation

This task is needed because:

Triggers for this work:

  • Technical debt accumulation
  • Maintainability concerns

Risks

Potential risks:

  • Risk 1: The umbrella scope expands indefinitely. — Mitigation: this issue is a gate for the per-function refactors, not a replacement. Each function gets its own PR (or its own sub-issue if it is not already covered).
  • Risk 2: Decomposition introduces too many small files, violating rule 0003's "second use site" threshold. — Mitigation: helpers are extracted only when they have a name that survives the function boundary, not for the sake of decomposition.

Migration Plan

Migration approach:

  1. Land the P0 and structure-issues that touch each function: [Refactor]: Replace chained cast in error.ts:130 with a single cast on the declared type #71 (chained cast in error.ts), [Refactor]: Remove silent try/catch around instanceof in is/index.ts #72 + [Refactor]: Remove redundant typeof/null guard after narrowing in is/index.ts #73 (silent catch and redundant guard in is/index.ts), [Refactor]: Decompose and rename ErrorFactoryInstance in error.ts #77 (rename + decompose ErrorFactoryInstance), [Feature]: Add .addNote() method to ErrorInstance #9 (extracted inheritance walk).
  2. After each lands, re-read the affected function top-down. Confirm the first line says what the function does and every subsequent line is a name the consumer follows.
  3. Close this issue once every public function in packages/errors/src/ passes the top-down reading test.

Rollback plan: revert any of the underlying PRs; this umbrella has no code of its own.

Backward Compatibility

  • This refactoring maintains full backward compatibility (no public API change)

Scope

Files/Folders affected:

  • All of packages/errors/src/ (read-through audit, plus the underlying refactors)

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
  • effort: s - Half a day
  • effort: m - 1-2 days
  • Unknown - needs investigation (umbrella scope, depends on the underlying refactors)

Test Coverage Requirements

  • Existing tests cover this code area (will update)
  • Need to add new tests for this refactor

Testing Approach

No new tests of its own. The underlying refactors (each with their own issue) bring their test coverage.

Related Issues / Pull Requests

Relevant Documentation

  • Architecture doc: docs/engineering/architecture/rules/0007-top-down-composition.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)

Activity

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

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