Skip to content

fix: handle malformed Core JSON responses#158

Merged
wizzomafizzo merged 2 commits into
mainfrom
fix/issue-147-malformed-core-json
Jun 3, 2026
Merged

fix: handle malformed Core JSON responses#158
wizzomafizzo merged 2 commits into
mainfrom
fix/issue-147-malformed-core-json

Conversation

@wizzomafizzo

@wizzomafizzo wizzomafizzo commented Jun 3, 2026

Copy link
Copy Markdown
Member

Closes #147

Summary

  • add a typed malformed Core response error with sanitized parse-failure context
  • reject matching pending JSON-RPC requests immediately when malformed frames include an id
  • keep media tags and message handling on recoverable UI error paths

Validated with focused CoreAPI, TagSelector, and ConnectionProvider tests plus typecheck and lint.

Summary by CodeRabbit

  • Tests

    • Expanded test coverage for malformed API responses, including notification handling and tag-loading error states.
  • Bug Fixes

    • Improved handling of malformed API responses: clearer user-facing error notifications, tag selector shows an "unavailable" state for malformed data, and recoverable malformed responses are logged as warnings to aid diagnostics.

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 499c1167-386b-4e4f-b559-b95b7e5d23a8

📥 Commits

Reviewing files that changed from the base of the PR and between 099b407 and a470f73.

📒 Files selected for processing (1)
  • src/lib/coreApi.ts

📝 Walkthrough

Walkthrough

Adds a MalformedCoreResponseError class and helpers; processReceived normalizes raw WebSocket payloads, constructs this error on JSON parse failure, routes it to matching pending response entries when possible, and logs warnings with metadata. Tests and component mocks are extended to assert rejection behavior and UI error handling.

Changes

Malformed Core Response Handling

Layer / File(s) Summary
MalformedCoreResponseError class and helpers
src/lib/coreApi.ts
New exported MalformedCoreResponseError and isMalformedCoreResponseError, plus helpers to normalize WebSocket msg.data, create sanitized/truncated previews, and extract JSON-RPC id from malformed text.
JSON parsing error handling and logging
src/lib/coreApi.ts
logMediaApiFailure now detects MalformedCoreResponseError and emits logger.warn with requestId, dataLength, and dataPreview.
processReceived routing to responsePool
src/lib/coreApi.ts
processReceived normalizes incoming data, builds a MalformedCoreResponseError on parse failure, extracts an id if present, and either rejects the matching responsePool pending promise (clearing its timeout and removing it) or rejects directly when no match exists.
Core API unit and contract tests
src/__tests__/unit/lib/coreApi.test.ts, src/__tests__/unit/coreApi.internals.test.ts, src/__tests__/unit/coreApi.api-contract.test.ts
Tests now assert processReceived rejects with MalformedCoreResponseError, verify pending-request rejection semantics when an id matches, and confirm logger.warn is called with contextual metadata for mediaTags() contract cases.
Component integration tests and mocks
src/__tests__/unit/components/ConnectionProvider.test.tsx, src/__tests__/unit/components/TagSelector.test.tsx
ConnectionProvider test asserts an error toast when CoreAPI.processReceived rejects; TagSelector tests and mocks include MalformedCoreResponseError and assert unavailable UI when media tags reject with the malformed error.

Sequence Diagram(s)

sequenceDiagram
  participant WebSocket
  participant CoreAPI as CoreAPI.processReceived
  participant ResponsePool
  participant Logger as logger.warn

  WebSocket->>CoreAPI: incoming raw message (msg.data)
  CoreAPI->>CoreAPI: normalize data, attempt JSON.parse
  CoreAPI-->>CoreAPI: on parse error, create MalformedCoreResponseError
  CoreAPI->>ResponsePool: lookup extracted requestId
  alt matching pending request
    ResponsePool->>ResponsePool: clear timeout, remove entry
    ResponsePool->>CoreAPI: reject pending promise with MalformedCoreResponseError
  else no match
    CoreAPI->>CoreAPI: reject with MalformedCoreResponseError
  end
  CoreAPI->>Logger: warn(..., { requestId, dataLength, dataPreview, action })
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • ZaparooProject/zaparoo-app#153: Modifies media error handling/logging paths in src/lib/coreApi.ts, related to how media API failures are logged and surfaced.

Poem

🐰 I found a JSON all askew,
Truncated bytes and missing glue.
Now parsed with care, and logged with light—
Warnings saved us from 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 directly and clearly describes the main change: introducing error handling for malformed Core JSON responses, which aligns with the primary objective of the PR.
Linked Issues check ✅ Passed All coding requirements from #147 are met: malformed JSON responses now throw a typed MalformedCoreResponseError, pending requests are immediately rejected when matching an id, and the error includes sanitized context without route-level crashes.
Out of Scope Changes check ✅ Passed All changes are scoped to handling malformed Core JSON responses in CoreAPI and related test coverage; no unrelated modifications were introduced.

✏️ 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-147-malformed-core-json

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

@codecov

codecov Bot commented Jun 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.30435% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/lib/coreApi.ts 91.30% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/coreApi.ts`:
- Around line 98-105: createSanitizedDataPreview currently builds a full
sanitized copy of data then slices it, which walks and allocates the entire
string; change it to stream and stop after producing 200 characters: iterate
through the input string using an index, use codePointAt to read each Unicode
code point and advance the index by the code point's UTF-16 length (1 or 2),
append either the original character(s) or a single space when the code point is
<=31 or ===127, and break once the output length reaches 200; update the
function name createSanitizedDataPreview and its implementation to avoid
Array.from/map/join and to return the built preview directly.
- Around line 148-158: The current catch path in the
isMalformedCoreResponseError branch logs a warning here (using logger.warn)
which causes duplicate reporting because processReceived already emits
logger.error and caller code like mediaTags() → logMediaApiFailure() also logs;
remove the logger.warn call in this branch and simply return (or rethrow) so
that the caller-specific catch path (e.g., mediaTags / logMediaApiFailure) is
the single place that logs the malformed-core error; apply the same change to
the other occurrence mentioned (lines 661-668) and ensure you reference
isMalformedCoreResponseError and processReceived to locate both spots.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7f36a49d-1929-4845-aa03-87e8febb8526

📥 Commits

Reviewing files that changed from the base of the PR and between 6f35871 and 099b407.

📒 Files selected for processing (6)
  • src/__tests__/unit/components/ConnectionProvider.test.tsx
  • src/__tests__/unit/components/TagSelector.test.tsx
  • src/__tests__/unit/coreApi.api-contract.test.ts
  • src/__tests__/unit/coreApi.internals.test.ts
  • src/__tests__/unit/lib/coreApi.test.ts
  • src/lib/coreApi.ts

Comment thread src/lib/coreApi.ts Outdated
Comment thread src/lib/coreApi.ts
@wizzomafizzo
wizzomafizzo merged commit bc4aeb2 into main Jun 3, 2026
6 checks passed
@wizzomafizzo
wizzomafizzo deleted the fix/issue-147-malformed-core-json branch June 3, 2026 08:26
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.

Handle malformed Core JSON responses without crashing

1 participant