Skip to content

fix(api): harden toError against circular-reference payloads#146

Merged
mpiton merged 2 commits intomainfrom
fix/issue-142-toError-circular
May 4, 2026
Merged

fix(api): harden toError against circular-reference payloads#146
mpiton merged 2 commits intomainfrom
fix/issue-142-toError-circular

Conversation

@mpiton
Copy link
Copy Markdown
Owner

@mpiton mpiton commented May 4, 2026

Summary

Hardens the toError error-normalization helper in src/api/client.ts against circular-reference payloads. Previously, a circular object rejected by invoke would throw TypeError: Converting circular structure to JSON from inside the tauriInvoke catch block, masking the original IPC failure. Closes #142.

Why

When a Rust error from Tauri IPC is enriched or re-wrapped in a way that creates a circular reference before rejection, JSON.stringify(err) synchronously throws a TypeError instead of succeeding. The uncaught throw occurs inside the tauriInvoke catch handler, replacing the original error with a normalization failure — unhelpful for debugging.

Changes

  • Wrap JSON.stringify(err) in try/catch in toError() function.
  • Fallback to String(err) when JSON serialization fails (covers circular refs and other stringify edge cases).
  • Three new regression tests: circular object rejection, plain-object stringification, string-rejection wrapping.
  • Update CHANGELOG.md under [Unreleased] > Fixed scope api.

Testing

npx vitest run src/api/__tests__/client.test.ts
# Test Files  1 passed (1)
# Tests  10 passed (10)

npx vitest run
# Test Files  82 passed (82)
# Tests  671 passed (671)

All TS linting and formatting pass. oxlint reports 0 warnings.

Related Issues

Checklist

  • Tests added and passing locally
  • CHANGELOG updated
  • No linting/formatting issues
  • Self-reviewed

Summary by cubic

Hardened toError to handle circular and non-serializable payloads so tauriInvoke surfaces the real IPC error instead of JSON errors or blank messages. Closes #142.

  • Bug Fixes
    • Guard JSON.stringify(err) and fall back to String(err); coalesce undefined stringify results to avoid blank Error.message.
    • Preserve string rejections; JSON-stringify plain objects.
    • Added tests for circular objects, plain-object JSON, string rejections, undefined, and function/symbol rejections.

Written for commit 18ec66f. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes

    • Improved API error handling to prevent serialization issues (including circular or otherwise unserializable payloads) from masking original error messages, ensuring meaningful, non-blank error text.
  • Tests

    • Added tests covering error rejection behavior across circular references, plain objects, strings, undefined and other edge payloads to guard against regressions.

toError in src/api/client.ts wraps JSON.stringify in try/catch and
falls back to String(err) when the rejected payload contains a circular
reference. Previously a circular err could throw TypeError: Converting
circular structure to JSON from inside tauriInvoke catch block, masking
the original failure with a normalization error.

Three regression tests cover circular objects, plain-object
stringification, and string-rejection wrapping.
@github-actions github-actions Bot added documentation Improvements or additions to documentation frontend labels May 4, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e62a7cef-e67f-4dd4-8c22-e5afb3450f03

📥 Commits

Reviewing files that changed from the base of the PR and between 9e0dbb2 and 18ec66f.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • src/api/__tests__/client.test.ts
  • src/api/client.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/api/client.ts
  • src/api/tests/client.test.ts
  • CHANGELOG.md

📝 Walkthrough

Walkthrough

The toError helper in src/api/client.ts is hardened against circular-reference and other non-stringifiable rejection payloads by wrapping JSON.stringify in a try/catch, explicitly handling string inputs, and falling back to String(err) when needed. Tests and CHANGELOG entry added to cover the behaviors.

Changes

Error Handling Robustness

Layer / File(s) Summary
Core Implementation
src/api/client.ts
toError now returns new Error(err) for string inputs, attempts JSON.stringify(err) inside a try/catch, and falls back to String(err) if stringification throws or yields undefined.
Tests
src/api/__tests__/client.test.ts
Added tests asserting circular-reference rejection payloads do not cause JSON/type errors, plain-object rejections are stringified, string rejections are rethrown as raw strings, and undefined/non-stringifiable payloads produce non-blank error messages.
Changelog
CHANGELOG.md
Unreleased → Fixed entry documents the defensive try/catch around JSON.stringify, fallback to String(err), and added regression tests for circular and edge-case rejection payloads.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I’m a rabbit who caught a loop in flight,
Wrapped it in try/catch and set it right.
Strings stay tidy, circles don’t bite,
Fallbacks shine softly in the night. 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: hardening the toError function to handle circular-reference payloads, which is the primary focus of the PR.
Linked Issues check ✅ Passed The PR fully addresses all coding requirements from issue #142: handles Error instances, string inputs, attempts JSON.stringify with try/catch, and falls back to String(err) to avoid throwing on circular references.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #142: modifications to toError in src/api/client.ts, test coverage for circular-reference and edge cases, and CHANGELOG documentation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-142-toError-circular

Review rate limit: 4/5 reviews remaining, refill in 12 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/api/client.ts`:
- Around line 7-10: The current error-wrapping code returns new
Error(JSON.stringify(err)) but doesn't handle JSON.stringify returning undefined
(e.g., for undefined, symbols, or functions), producing a blank Error.message;
update the block in src/api/client.ts that handles err so you first call const
msg = JSON.stringify(err); if msg === undefined then use String(err) (or
fallback like Object.prototype.toString.call(err)) and finally return new
Error(msgOrFallback) while keeping the existing catch fallback for thrown
stringify errors.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cc9e6104-f15f-4666-8793-daf34d622df3

📥 Commits

Reviewing files that changed from the base of the PR and between 53b272f and 9e0dbb2.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • src/api/__tests__/client.test.ts
  • src/api/client.ts

Comment thread src/api/client.ts
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/api/client.ts">

<violation number="1" location="src/api/client.ts:8">
P3: Handle the `JSON.stringify` non-throwing `undefined` case before constructing the `Error`; otherwise some rejected payloads produce an empty error message.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread src/api/client.ts Outdated
@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq Bot commented May 4, 2026

Merging this PR will not alter performance

✅ 26 untouched benchmarks


Comparing fix/issue-142-toError-circular (18ec66f) with main (53b272f)

Open in CodSpeed

JSON.stringify returns undefined (not a string) for bare undefined,
functions, and symbols. The previous fix only guarded against the throw
path; an undefined return would still construct new Error(undefined),
producing a blank Error.message and masking the original failure.

Coalesce with ?? String(err) so the fallback path is shared with the
catch branch. Adds two regression tests covering the bare undefined
rejection and the function/symbol rejection paths.

Addresses PR #146 review feedback from coderabbitai and cubic-dev-ai.
@mpiton mpiton merged commit 7ecd9b0 into main May 4, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation frontend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(api): harden toError against circular-reference payloads

1 participant