fix(webhooks): read every Ashby error shape when webhook registration fails - #6711
Conversation
… fails
The provider read errorInfo.message and a top-level message, but not the
`errors` array. Ashby uses three shapes in practice, confirmed live: objects
`[{ message, parameter }]`, plain strings `['webhook_not_found']`, and
`errorInfo`. A missing apiKeysWrite permission arrives in the array form, so
the user saw 'Unknown Ashby API error' instead of the cause.
The duplicate-webhook branch made it worse: it only fires when the message was
extracted, so an unparsed error also cost the user the one actionable
instruction for fixing it - delete the duplicate under Settings > API/Webhooks.
Uses the shared ashbyErrorMessage extractor rather than a second partial copy,
matching how other providers already import from @/tools. The delete path now
reports why it failed instead of only the HTTP status.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Delete no longer treats HTTP 200 as success when the body has Adds focused Vitest coverage for create/delete error paths. Trigger setup text clarifies apiKeysWrite, deploy-vs-save timing, and adds Temporary to job employment type descriptions. Reviewed by Cursor Bugbot for commit f1a72f4. Configure here. |
Greptile SummaryThe PR expands Ashby webhook error handling and deletion-result parsing while correcting trigger setup guidance.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/webhooks/providers/ashby.ts | Adds comprehensive Ashby error extraction and response-aware, idempotent webhook deletion handling without an eligible follow-up defect. |
| apps/sim/lib/webhooks/providers/ashby.test.ts | Adds coverage for all observed error envelopes, duplicate-webhook guidance, strict deletion failures, not-found responses, and successful deletion. |
| apps/sim/triggers/ashby/utils.ts | Corrects setup timing and permission guidance and documents the Temporary employment type. |
Reviews (4): Last reviewed commit: "fix(webhooks): match Ashby's real not-fo..." | Re-trigger Greptile
…onfig CI runs `biome check .` from apps/sim; I had run biome ad hoc from the repo root, which resolves a different config and left this hunk unformatted.
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit db5dee4. Configure here.
Importing the shared extractor from @/tools/ashby/utils failed check:tool-registry-boundary. Two separate reasons, both real: An import edge from lib/webhooks/providers into @/tools/** grows the workspace page graphs that reach the providers, because @/tools/types statically reaches @/lib/oauth, the rate limiter and the executor. And carving the helper into its own file did not help either: the knowledge page graph already sits exactly at the +42 ceiling the audit allows, so one more module anywhere it can reach is one too many. So the logic is duplicated across the subsystem boundary rather than shared across it, with a comment on both sides saying why. Both copies derive from the same three documented Ashby error shapes and are covered independently.
Ashby returns what would be a 4XX elsewhere as HTTP 200 with `success: false` — its own docs state this explicitly. `deleteSubscription` branched on `ashbyResponse.ok`, so every rejected delete logged "Successfully deleted Ashby webhook subscription <id>" and never threw in strict mode. Sim then dropped its own row while the subscription stayed live in Ashby, and since there is no `webhook.list` endpoint the orphan cannot be enumerated afterwards. Check `success` the way `createSubscription` already does, and treat `webhook_not_found` as already-removed rather than an error — that is the shape an unknown id comes back in, not a 404. An absent `success` field stays a success here, unlike on create: teardown runs on the undeploy path, and failing closed on an undocumented response shape would wedge cleanup. Also corrects two trigger-surface details against the API reference: the setup text said the webhook is created when you save the trigger (it is created on deploy), and the jobCreate `employmentType` description omitted the documented `Temporary` value.
|
@cursor review |
The already-removed branch tested `/webhook_not_found/` against the extracted message, but `ashbyErrorMessage` returns `errorInfo.message` first and that reads "Webhook not found" — Ashby carries the machine code on `errorInfo.code` and in the deprecated `errors` array, both of which lose to the message. So the one envelope this branch exists for, a repeat delete of an id Ashby has already dropped, fell through to the failure path: a spurious warn today and a strict-mode throw on the undeploy cleanup path. Read the codes directly and keep a prose fallback for the message-only form. Caught by Cursor Bugbot.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f1a72f4. Configure here.
2 participants
Summary
Follow-up to #6703, found while testing the Ashby webhook triggers end to end against a real organization.
The webhook provider read
errorInfo.messageand a top-levelmessage, but not theerrorsarray. Ashby uses three error shapes in practice, all confirmed live:errors: [{ message, parameter }]errors: ["webhook_not_found"]webhook.deleteon an unknown iderrorInfo: { code, message, requestId }A key missing
apiKeysWritefails in the array form, so registering a trigger showed "Unknown Ashby API error" instead of the actual cause.The duplicate-webhook branch made that worse. It only fires when the message was successfully extracted:
So an unparsed error also cost the user the one actionable instruction for recovering — delete the duplicate under Ashby Settings > API/Webhooks — and they got a generic failure instead.
Approach
Uses the shared
ashbyErrorMessageextractor from@/tools/ashby/utilsrather than keeping a second partial copy in the provider. That helper already handles all three shapes and is covered by its own tests, so there is one definition of "how to read an Ashby error" instead of two that drift.Checked the import boundary first:
linq,clickup,zoho-desk,gitlab, andemailbisonproviders already import from@/tools, so this follows established precedent rather than introducing a new dependency direction.The delete path now reports why it failed instead of only the HTTP status.
Testing
Four new tests cover each error shape and the duplicate-guidance path, which previously had no coverage for
createSubscriptionat all. 21 passing in the provider suite.Verified end to end against a real Ashby organization using a cloudflare tunnel, with the
job createdtrigger (job metadata only — no candidate data was ever delivered):webhook.createregistered successfully (registration_status: active, realexternalId)jobCreatepayload delivered to the registered URL returned200 Webhook processedtrigger=ashby, status=completed)401 Unauthorized, so verification was proven to reject as well as acceptwebhook_not_foundNo residue: the webhook is gone from Ashby, local webhook rows are 0.
Notes for reviewers
The receive path needed no changes. It uses HMAC-SHA256 with a timing-safe
safeCompareand rejects both a missing secret and a missing signature.Two things worth knowing that are not changed here:
webhook.listdoes not exist in Ashby's API. Teardown can only be verified by attempting a delete twice, which is what the test above did. Worth knowing if a webhook ever leaks, since you cannot enumerate to find it.candidatewebhook row; deploying promotes it toactiveand callswebhook.create. The block's setup text says the webhook is created "when you save the trigger", which is misleading. Left alone here to keep this diff focused.