fix(calendar): surface curated provider errors in the calendar tooltip - #1868
giladresisi wants to merge 3 commits into
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
| const failureMessage = | ||
| err?.cause?.failure?.message || err?.failure?.cause?.message; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Not a bug — changeState is an activity, so err arrives through Temporal's payload serialization, not as a live ApplicationFailure. In the serialized form the message is at cause.failure.message (see the shape stored in existing production post.error rows). Verified e2e before opening this PR: a real Pinterest "Board not found" failure stored {"message":"The specified board was not found. Please check the board ID."} and rendered in the tooltip, which couldn't happen if this path were always undefined.
| const failureMessage = | ||
| err?.cause?.failure?.message || err?.failure?.cause?.message; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Not a bug, and this is the same claim as the earlier comment on this line.
changeState runs as a Temporal activity, so err arrives as a serialized payload rather than a live ApplicationFailure. In that serialized form the message sits at cause.failure.message, which is why the extra hop is there. Switching to err?.cause?.message as suggested would break the path that currently works.
Confirmed again today with a second real case, independent of the Pinterest one: a TikTok video post failed through the real workflow with a mapped provider error, and the stored column was {"message":"Media size not supported by TikTok: images up to 1080px on the shorter side, videos at least 360px on both sides"}. That is the curated format written by this exact code, which could not happen if failureMessage were always undefined.
The calendar error tooltip always showed the generic 'An error
occurred while publishing this post' because getPosts never selected
the error column. Select it, but surface ONLY curated provider
messages: bad_body failures whose message came from a provider
handleErrors mapping or a hand-written provider throw. Unmapped API
responses carry the 'Unknown Error' placeholder at the point of
throwing, so anything else - raw API bodies, unexpected exceptions,
internal workflow strings like 'Refresh channel needed' - keeps the
generic tooltip exactly as today.
Curated messages are stored in post.error as {"message":"..."} so
the frontend can distinguish them from everything the column held
until now (internal strings, serialized ActivityFailure dumps), which
all render generic. Internal strings are still stored for debugging
and the raw failure still goes to the Errors table. The
checkPostStatus/finalizePost developer-guard throws are demoted to the
unmapped placeholder (detail moved to the json field) so they can
never surface either. The tooltip is capped at 400px with wrapping.
Verified e2e through real Temporal v1.0.6 workflows on a local run:
a real Pinterest 'Board not found' response surfaced its handleErrors
mapping; an unmapped bad_body, an unexpected exception, a
refreshNeeded early-exit string, and a seeded legacy serialized-blob
row all rendered the generic tooltip (checked in the rendered
calendar DOM).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…edly changeState only wrote the error column when it had a curated message to store, so a post that first failed with a curated bad_body message and then failed again for an uncurated reason (a token refresh failure, an unexpected exception) kept the old value. The calendar tooltip then showed the earlier, unrelated error - exactly the misleading-message problem this branch set out to fix. Overwrite the column on every failure instead: curated messages and internal strings are stored as before, and anything uncurated clears it so the tooltip falls back to the generic text. The full failure is still recorded in the Errors table, so nothing is lost for debugging. Reported by Sentry's PR review bot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
e0aa9cb to
ae74540
Compare
Strix Security ReviewNo security issues found. Updated for Reviewed by Strix |
| const errorMessage = !err | ||
| ? undefined | ||
| : typeof err === 'string' | ||
| ? err // stored for debugging, rendered as the generic tooltip | ||
| : err?.cause?.type === 'bad_body' && | ||
| failureMessage && | ||
| failureMessage !== 'Unknown Error' | ||
| ? JSON.stringify({ message: failureMessage }) | ||
| : undefined; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Confirmed this was real in the intermediate commits after the staging rebase (local errorMessage shadowed the logger import in the errors.create catch), but f90e632 removed the local variable entirely when curation moved to read time, so the head no longer has it.
Post.error keeps persisting the raw serialized failure; curation moved to getPosts so the calendar and public API both get the curated message or null, retroactively for existing rows.
| ? [parsed?.cause?.type, parsed?.cause?.failure?.message] | ||
| : [ | ||
| parsed?.failure?.cause?.applicationFailureInfo?.type, | ||
| parsed?.failure?.cause?.message, | ||
| ]; | ||
| return type === 'bad_body' && message && message !== 'Unknown Error' |
There was a problem hiding this comment.
Bug: The curatedError function uses inconsistent paths to extract the error type and message from nested objects, which will likely cause type checks like type === 'bad_body' to fail.
Severity: MEDIUM
Suggested Fix
Align the data extraction paths in the curatedError function. When parsing the error, ensure the type is extracted from the same nested ApplicationFailure object as the message. For example, use a path like parsed?.cause?.failure?.applicationFailureInfo?.type to retrieve the type, consistent with how other parts of the function handle nested failures.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
libraries/nestjs-libraries/src/database/prisma/posts/posts.repository.ts#L143-L148
Potential issue: In the `curatedError` function, there is an inconsistent data
extraction path for handling nested errors from Temporal workflows. When
`parsed?.cause?.failure?.message` exists, the code extracts the error `type` from
`parsed?.cause?.type` but the `message` from the more deeply nested
`parsed?.cause?.failure?.message`. This creates a mismatch, as the `type` will likely
belong to a wrapper error (e.g., `ActivityFailure`) rather than the intended
`ApplicationFailure` (e.g., `bad_body`). As a result, the check `type === 'bad_body'`
will fail, preventing specific errors like `BadBody` from being correctly identified and
handled.
There was a problem hiding this comment.
Not a bug. curatedError parses the serialized failure stored in Post.error, not a live ActivityFailure, and in that serialized shape type sits at cause.type as a sibling of cause.failure (visible in existing production rows). This exact path was e2e-verified on this branch: a real mapped Pinterest failure and a legacy blob both returned their curated message through this check, and a crash-shaped blob returned null. The suggested cause.failure.applicationFailureInfo.type also exists in the blob, but the current path is the verified one.
…sage Facebook rejects page publishes with an OAuthException code 200 when the connected account lacks pages_manage_posts / pages_read_engagement or sufficient page role. This previously fell through handleErrors to the 'Unknown Error' placeholder, so the failure email (and, once merged, the curated calendar tooltip from gitroomhq#1868) showed nothing actionable. Per Meta's docs, codes 200-299 are API Permission errors, so the (gitroomhq#200) marker always denotes a permissions problem. As a side effect, the preset retry helper no longer treats these as 'Unknown Error', avoiding a wasted second publish attempt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Closing in favor of #2119. The branch was rebased onto main per the current base-branch policy, and the replacement now also exposes the curated error in the MCP List Posts tool. No other changes. |
What kind of change does this PR introduce?
Bug fix / UX improvement
Why was this change needed?
Failed posts show a red error badge in the calendar, but the tooltip always says the generic "An error occurred while publishing this post" — even when the provider produced an actionable, human-written message like "The specified board was not found. Please check the board ID.". The same gap exists in the public API:
GET /public/v1/postsreturns failed posts with onlystate: "ERROR"and no reason at all — a customer whose posts were failing due to a platform-side account restriction had no way to see why through the API, and the actual cause was only visible to us in internal tables.This is a reworked revival of #1803, which was closed because it surfaced every stored error. This version surfaces only curated provider messages, and does it at read time:
Post.errorkeeps persisting the raw serialized failure exactly as before (the DB write path is unchanged), and the sharedgetPostspath replaces the value with the curated message or null. Curated means abad_bodyfailure whose message came from a providerhandleErrorsmapping or a hand-written provider throw; unmapped API responses carry the'Unknown Error'placeholder at the throw site, so raw API bodies, unexpected exceptions, and internal workflow strings ('Refresh channel needed') all come out as null — the calendar keeps showing the generic tooltip for those, exactly as today. ThecheckPostStatus/finalizePostdeveloper-guard throws are demoted to the unmapped placeholder so they can never surface either. Emails and in-app notifications are untouched.Because
getPostsalso servesGET /public/v1/posts, the public API now returns the same curated-or-nullerrorfield. Read-time extraction is also retroactive: posts that already errored get the curated message with no migration, since the mapped message lives inside the stored failure blob. Tooltip is capped at 400px with wrapping.Verified e2e through real Temporal workflows on a local run, checking all three surfaces per case (DB row, rendered calendar DOM, public API response):
errorOther information:
Replaces the closed #1803; follow-up to the investigation behind #1802/#1804.
Companion docs PR: gitroomhq/postiz-docs#243 documents the new
errorfield in the Public API List Posts response — it should be merged only after this PR deploys.Checklist:
🤖 Generated with Claude Code