feat(ipni): expose failureReason as top-level field on ipni_tracking_failed#491
Merged
Merged
Conversation
…failed toStructuredError(error) only captures error.message inside a nested error.message field, which forces BS users to JSON-extract through two layers to filter or aggregate by failure type. Promote the underlying message to a top-level failureReason field so operators and SPs can group ipni_tracking_failed events by reason class without parsing the stack-bearing error blob. Companion to the fix in #490.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances backend IPNI failure observability by promoting the underlying failure message into a top-level failureReason field on the ipni_tracking_failed structured log event, making it easier to filter/aggregate in log analytics without nested JSON extraction.
Changes:
- Add
failureReasonas a top-level field on theipni_tracking_failedlog event (derived from the caught error’s message). - Keep the existing
error: toStructuredError(error)payload unchanged (additive change only).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…lureReason Companion to #490, which now carries the actual failure detail via Error.cause rather than concatenating it into the outer message. Promote cause.message to the top-level failureReason field when present, falling back to error.message when no cause is attached (e.g. for non-rethrown errors thrown directly inside the catch block). Result: { failureReason: "IPNI verification timed out after 60000ms" } instead of { failureReason: "IPNI verification failed for deal X: root CID not verified" }.
4 tasks
juliangruber
approved these changes
May 4, 2026
github-project-automation
Bot
moved this from 🔎 Awaiting review
to ✔️ Approved by reviewer
in FOC
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
failureReasonfield to theipni_tracking_failedstructured log alongside the existingerror: toStructuredError(error)bloberror.messageWhy
toStructuredError(error)nests the underlying message insideerror.message, which means every BS query that wants to group by reason has to drill into the nested struct. Promoting the message to a top-level field is friction-free for operators and SPs.Companion to #490 (which makes the underlying message carry the actual reason instead of "root CID not verified").
See dealbot#473 audit comment for the full investigation: #473 (comment)
Test plan
pnpm exec vitest run src/deal-addons/strategies/ipni.strategy.spec.ts-> 11/11 passfailureReasonis additiveWHERE event = 'ipni_tracking_failed' GROUP BY failureReasonshould bucket failures cleanly without parsing nested fields