Skip to content

Log error cause property#19

Merged
Macil merged 2 commits into
masterfrom
chris/betterLogging
Mar 30, 2026
Merged

Log error cause property#19
Macil merged 2 commits into
masterfrom
chris/betterLogging

Conversation

@Macil
Copy link
Copy Markdown
Contributor

@Macil Macil commented Mar 30, 2026

The cause property of Errors was not logged. This property is used by the retry function from @std/async to report the original error when a retry limit is hit.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Mar 30, 2026

Greptile Summary

This PR fixes a gap in the logging serialization layer where the cause property of Error objects was never included in log output. This matters because @std/async's retry utility wraps the original error in cause when the retry limit is hit, so previously that root cause was silently swallowed.

Key changes:

  • A new indentLines.ts utility is extracted to indent multi-line strings (used to visually nest the cause under its parent error).
  • serialization.ts moves the Error branch from before the circular-reference check into the try block after stack.push(value). This is a meaningful correctness improvement: it means a circularly referenced Error (e.g. err.cause = err) is now correctly detected and printed as [Circular reference] instead of recursing infinitely.
  • The cause is appended in a { cause: ... } block after the error's stack trace, and serializeValue is called recursively so the cause itself benefits from the same serialization logic (including its own nested cause, circular detection, etc.).
  • Tests cover the normal cause case and the self-referential/circular case.

One thing to be aware of: the guard if (cause) uses a truthy check, which means falsy-but-set cause values (null, false, 0, "") are silently omitted. In practice cause is nearly always an Error or string, so this is unlikely to bite the current use case, but if (cause !== undefined) would be the fully correct guard per the ECMAScript spec.

Confidence Score: 5/5

  • Safe to merge; the only finding is a minor edge-case with falsy cause values that won't affect the primary use case
  • All changes are well-tested and logically correct for the stated use case. The single P2 finding (truthy vs. undefined check for cause) is a minor correctness gap for uncommon falsy cause values and does not affect real-world usage or the motivation for this PR.
  • logging/serialization.ts — the cause guard on line 33

Important Files Changed

Filename Overview
logging/serialization.ts Moves Error handling into the circular-reference-aware try block, adds cause serialization; minor issue: truthy check silently drops falsy cause values like null/false/0
logging/indentLines.ts New utility that indents non-empty lines of a multi-line string; simple and correct
logging/serialization.test.ts Adds two new tests: one for a normal cause and one for a self-referential cause (circular reference detection); both are well-structured
logging/indentLines.test.ts New unit tests for indentLines covering basic case and empty-line passthrough; complete coverage for the small utility

Reviews (1): Last reviewed commit: "log error cause property" | Re-trigger Greptile

Comment thread logging/serialization.ts Outdated
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new indentLines utility and enhances the serializeValue function to recursively serialize the cause property of Error objects. The feedback suggests improving the robustness of the cause check by explicitly checking for undefined to ensure falsy values like 0 or false are correctly processed.

Comment thread logging/serialization.ts Outdated
@Macil Macil merged commit 5c45613 into master Mar 30, 2026
1 check passed
@Macil Macil deleted the chris/betterLogging branch March 30, 2026 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant