Fall back when Linked Data signing hits JSON-LD processing errors#842
Conversation
✅ Deploy Preview for fedify-json-schema ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds JSON-LD signing error classification and changes outbound RSA Linked Data signing to log matching failures and continue without a Linked Data signature. Also adds a changelog note for the release. ChangesLinked Data signature fallback
Sequence Diagram(s)sequenceDiagram
participant sendActivity
participant signJsonLd
participant jsonld.canonize
participant isLinkedDataSignatureJsonLdProcessingError
sendActivity->>signJsonLd: create RSA Linked Data signature
signJsonLd->>jsonld.canonize: canonize JSON-LD
jsonld.canonize-->>signJsonLd: throw processing error
signJsonLd-->>sendActivity: raise error
sendActivity->>isLinkedDataSignatureJsonLdProcessingError: classify caught error
isLinkedDataSignatureJsonLdProcessingError-->>sendActivity: true for matching JSON-LD failures
Note over sendActivity: log warning and continue without Linked Data signature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request ensures that outbound activity delivery does not abort when Linked Data Signature creation fails during JSON-LD canonicalization. Instead, it logs a warning and continues delivery without the signature. The review feedback points out a potential runtime crash in the new isLinkedDataSignatureJsonLdProcessingError helper function, where calling .startsWith() on potentially undefined properties (error.message or error.name) could throw a TypeError. Using optional chaining is recommended to ensure robust error handling.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/fedify/src/federation/middleware.ts`:
- Around line 425-439: Update the JSON-LD error classification in the middleware
so signing/configuration failures remain fatal: the logic in federation
middleware should inspect the actual nested failure on error.cause for
jsonld.InvalidUrl instead of error.details.cause, and it should not treat
UnsafeJsonLdError as retryable/continuable. Adjust the predicate around the
existing jsonld.InvalidUrl handling and the final fallback in the middleware
helper that classifies errors so Unsupported JSON-LD keyword and related
jsonld.* failures still propagate as aborting errors rather than
warning-and-continuing.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cc7a4393-9304-47fe-826f-15296c8769d8
📒 Files selected for processing (2)
CHANGES.mdpackages/fedify/src/federation/middleware.ts
Codecov Report❌ Patch coverage is
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
| isInvalidUrlTypeError(error))); | ||
| } | ||
|
|
||
| function isLinkedDataSignatureJsonLdProcessingError( |
There was a problem hiding this comment.
I think this could be simplified by avoiding access to unknown fields throughout the body, and handling it with a single structural type. just a thought
There was a problem hiding this comment.
It might be better to split it into two functions: a user-defined type guard which takes an unknown value, and an actual predicate function which takes a proper structural type.
dahlia
left a comment
There was a problem hiding this comment.
Could you change the target of this pull request to 2.0-maintenance rather than main?
So, could you put |
Assisted-by: Codex:gpt-5.5
Assisted-by: Codex:gpt-5.5
Assisted-by: Codex:gpt-5.5
|
@dahlia I’ve updated all the points you mentioned. Could you please check them? Thank you! |
Description
Fixes #824
This changes outbound activity delivery to continue without an optional Linked Data Signature when JSON-LD canonicalization/processing fails during signing. These failures are logged as warnings.
Other signing failures, such as key, configuration, or programming errors, still fail.
AI Usage
Codex (GPT-5.5) was used after the implementation for self-review.