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)

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

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions