Skip to content

[Refactor]: Decompose and rename ErrorFactoryInstance in error.ts #77

Description

@martyy-code

Current State

In packages/errors/src/error/error.ts:87, the inner function returned by error() is named ErrorFactoryInstance. The name has two problems:

  1. It is misleading. The function is not an instance — it is the factory that produces instances. The ErrorFactory type (the contract) and ErrorFactoryInstance (this internal function) are conflated.
  2. It violates two rules:
    • Rule 0013 (Entity-First Naming): the name ErrorFactoryInstance is a -er shape (Factory) with a redundant suffix. The rule's exception list (test names, brief variables) does not cover local function names.
    • Rule 0016 (No Generic Verbs): the name does not encode the transformation. ErrorFactoryInstance is "factory instance" — a noun phrase, not a verb-noun phrase. A reader cannot tell from the name what the function does, only what it is a kind of.

The function body is 50 lines (lines 87-134) and does several things: format the message, capture the stack, build the instance, attach the cause chain method, attach the addNote method, and mark the factory. A single function doing six things under a generic name is the exact pattern rule 0005 / 0007 warn against.

Located in:

  • packages/errors/src/error/error.ts:87-134

Proposed State

After refactoring, the inner function is renamed to encode the transformation, and its body is decomposed into named operations consistent with rule 0007 (top-down composition):

export const error = <const T extends Record<string, unknown> = Record<string, never>>(config: {
  name: string;
  fields?: StandardSchemaV1;
  inherits?: ErrorFactory | ErrorFactory[];
  message?: string;
}): ErrorFactory<T> => {
  const { name, fields, inherits, message } = config;

  const createError = (input?: Partial<T>): ErrorInstance<T> => {
    const fieldsData = (input || {}) as T;
    const instance = buildErrorInstance(name, message, fieldsData);
    attachFactoryMarker(instance, createError);
    return instance;
  };

  // ... metadata attachment ...
  return createError as ErrorFactory<T>;
};

Where buildErrorInstance, attachFactoryMarker, and the message-formatting helper are extracted into the renamed message-template.ts (see P0 #6) and stack-trace.ts files.

Expected improvements:

  • Rule 0013 compliance: the function is named for what it does (createError), not what it is a kind of.
  • Rule 0016 compliance: the verb (create) is specific; the return type (ErrorInstance<T>) is the result.
  • Rule 0007 compliance: the function reads top-down — the first line is the outcome, every subsequent line is a name the reader follows.
  • Rule 0005 compliance: the algorithm is named, not inlined.

Motivation

This refactoring is needed because:

  • The function name is a smell that the rules explicitly forbid; the library should not be a counter-example to its own rules.
  • The function body does six things; decomposing them is the kind of refactor that pays for itself the next time a feature is added.
  • The consumer-facing API (error({ name, fields, inherits, message })) does not change; the refactor is internal.

Triggers for this work:

  • Technical debt accumulation
  • Maintainability concerns

Risks

Potential risks:

  • Risk 1: Renaming the inner function affects the FACTORY_SYMBOL marker, which is checked at runtime in is/index.ts. — Mitigation: the rename is local; the marker is attached by reference (ErrorFactoryInstance becomes createError); the is() consumer side is unaffected because it only compares references.
  • Risk 2: Splitting the body into multiple functions introduces closure overhead. — Mitigation: V8 inlines aggressively for short closures; the named functions are pure utilities with no closure dependencies. Performance is unaffected.
  • Risk 3: The as ErrorFactory<T> cast at the return site requires care. — Mitigation: the cast is already present; the refactor preserves the same shape.

Migration Plan

Migration approach:

  1. Rename ErrorFactoryInstance to createError (or buildError, depending on the convention chosen).
  2. Extract buildErrorInstance(name, message, fieldsData) as a top-level function in message-template.ts or a new error/build.ts.
  3. Extract attachFactoryMarker(instance, factory) as a top-level function in the same file.
  4. Update the FACTORY_SYMBOL assignment to use the new name.
  5. Run the test suite; the public API is unchanged.

Rollback plan: revert the PR.

Backward Compatibility

  • This refactoring maintains full backward compatibility

Scope

Files/Folders affected:

  • packages/errors/src/error/error.ts (rename + extract)
  • packages/errors/src/error/types.ts (if new helpers are added)
  • packages/errors/src/is/index.ts (FACTORY_SYMBOL reference, no behaviour change)

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

Test Coverage Requirements

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

Testing Approach

Testing strategy:

  • Unit tests: existing error() test cases (single, multiple inheritance, with/without fields, with/without message template) should all pass.
  • New test: confirm that the FACTORY_SYMBOL marker is attached by the new attachFactoryMarker helper, and that is(err, factory) still discriminates correctly.

Verification steps:

  1. pnpm --filter @deessejs/errors test:run
  2. pnpm --filter @deessejs/errors type-check
  3. pnpm --filter @deessejs/errors build — public dist/ output matches the previous version's public types.

Related Issues / Pull Requests

Relevant Documentation

  • Architecture doc: docs/engineering/architecture/rules/0013-entity-first-naming.md
  • Architecture doc: docs/engineering/architecture/rules/0016-no-generic-verbs.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

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions