Skip to content

[WIKI-704] fix: hocuspocus error handling#7898

Merged
pushya22 merged 2 commits intopreviewfrom
fix/hocuspocus-error-handling
Oct 3, 2025
Merged

[WIKI-704] fix: hocuspocus error handling#7898
pushya22 merged 2 commits intopreviewfrom
fix/hocuspocus-error-handling

Conversation

@Palanikannan1437
Copy link
Member

@Palanikannan1437 Palanikannan1437 commented Oct 3, 2025

Description

Fixes a bug with html validation with tiptap v3 while saving html

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Summary by CodeRabbit

  • Bug Fixes
    • HTML sanitization now preserves the xmlns attribute across all tags, preventing unintended stripping of valid SVG/XML content.
    • Improved reliability when fetching and updating documents by standardizing error handling, reducing unexpected crashes and providing clearer error messages.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 3, 2025

Walkthrough

Adds xmlns to allowed HTML attributes in the API sanitizer. In the live app, centralizes error normalization in database extension and adjusts page service to rethrow full errors instead of response data. Also ensures an awaited update call in document storage.

Changes

Cohort / File(s) Summary of Changes
API HTML sanitizer
apps/api/plane/utils/content_validator.py
Allow xmlns in the global wildcard (*) ATTRIBUTES for HTML sanitization.
Live app error handling
apps/live/src/extensions/database.ts, apps/live/src/services/page/core.service.ts
Introduce internal normalizeToError helper; wrap thrown errors with contextual messages in fetch/store paths; await update call; page service now rethrows full error objects instead of only error.response.data in two methods.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Ext as extensions/database.ts
  participant Service as page/core.service.ts
  participant API as Backend API

  rect rgba(220,235,255,0.5)
  note over User,Ext: Fetch document flow
  User->>Ext: fetchDocument(pageId)
  Ext->>Service: fetchDescriptionBinary(pageId)
  Service->>API: GET /pages/:id/description
  API-->>Service: 200/4xx/5xx
  alt success
    Service-->>Ext: data
    Ext-->>User: document
  else error
    Service-->>Ext: throw Error (full error)
    Ext->>Ext: normalizeToError(e, "Failed to fetch document: :id")
    Ext-->>User: throw Error
  end
  end

  rect rgba(220,255,220,0.5)
  note over User,Ext: Store document flow
  User->>Ext: storeDocument(pageId, payload)
  Ext->>Service: updateDescriptionBinary(pageId, payload) (await)
  Service->>API: PUT /pages/:id/description
  API-->>Service: 200/4xx/5xx
  alt success
    Service-->>Ext: result
    Ext-->>User: ok
  else error
    Service-->>Ext: throw Error (full error)
    Ext->>Ext: normalizeToError(e, "Failed to update document: :id")
    Ext-->>User: throw Error
  end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

ready to merge

Suggested reviewers

  • sriramveeraghanta
  • dheeru0198
  • pablohashescobar

Poem

A nibble of attrs, a hop through the try—
I bundle the errors, no bytes left to fly.
With xmlns welcomed, the tags feel at ease,
I thump out a log and normalize, please.
Carrot-shaped commits, crisp as the air—
Docs fetched, stored, with careful hare-care. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The description provides the bug fix summary and marks the change type but omits any mention of the error handling updates and leaves out the Screenshots, Test Scenarios, and References sections required by the template. Please expand the Description to cover the new error handling logic, fill in the Test Scenarios section with how you verified both fixes, and add any relevant issue links or references under their respective headings.
Title Check ⚠️ Warning The pull request title focuses on “hocuspocus error handling” but the main change is adding xmlns support to the HTML sanitizer to fix a tiptap v3 validation bug, along with secondary error propagation updates in database and page services, so the title does not accurately summarize the primary fix. Rename the PR title to reflect the core HTML validation fix, for example “fix: add xmlns to HTML sanitizer for tiptap v3 validation”, or split the error handling improvements into a separate pull request if they address a different concern.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/hocuspocus-error-handling

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Contributor

@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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 794271a and 26f265a.

📒 Files selected for processing (3)
  • apps/api/plane/utils/content_validator.py (1 hunks)
  • apps/live/src/extensions/database.ts (3 hunks)
  • apps/live/src/services/page/core.service.ts (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
apps/api/plane/utils/content_validator.py (1)

87-87: LGTM! xmlns addition aligns with tiptap v3 requirements.

Adding xmlns to the wildcard attributes is appropriate for supporting tiptap v3's HTML output. The attribute is safe within the nh3 sanitization context and does not introduce security risks.

apps/live/src/services/page/core.service.ts (2)

23-25: LGTM! Full error propagation preserves context.

Rethrowing the complete error object instead of just error?.response?.data ensures stack traces and error metadata are preserved, improving debuggability.


37-39: LGTM! Consistent with fetchDetails error handling.

This change mirrors the error handling improvement in fetchDetails, ensuring both methods propagate full error context consistently.

apps/live/src/extensions/database.ts (3)

14-22: LGTM! Robust error normalization utility.

The normalizeToError helper ensures consistent Error objects across error paths, improving type safety and debuggability by preserving existing Error instances while converting other types to Error with meaningful messages.


40-43: LGTM! Enhanced error context for fetch failures.

Using normalizeToError with a descriptive fallback message improves error reporting when fetching documents fails, making it easier to diagnose issues in production.


59-62: LGTM! Consistent error handling in store operation.

The error normalization with context message aligns with the fetchDocument pattern, providing consistent error reporting across both database operations.

@Palanikannan1437 Palanikannan1437 changed the title fix: hocuspocus error handling [WIKI-704] fix: hocuspocus error handling Oct 3, 2025
@makeplane
Copy link

makeplane bot commented Oct 3, 2025

Linked to Plane Work Item(s)

This comment was auto-generated by Plane

@pushya22 pushya22 merged commit f6677f2 into preview Oct 3, 2025
7 checks passed
@pushya22 pushya22 deleted the fix/hocuspocus-error-handling branch October 3, 2025 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants