Repository navigation
fix(plugin-webhooks): report a webhook delivery the outbox could not record at error, once per webhook - #24862
Conversation
…t error, once per webhook A failed sys_http_delivery write loses the event for good (never sent, retried or redeliverable) while the record write already answered 200 and the webhook reads active. It was a bare WARN per event. It is now an error said once per webhook, with the consequence and the fix, re-armed by the next recorded delivery; later losses stay at debug with their eventId. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
…-webhook-enqueue-circular
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 4 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 37948a76aa8738e0045034030e5eee5270005312 && git checkout 37948a76aa8738e0045034030e5eee5270005312
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin df8bb40a032c016276e0b16c5edbd90cad086c9d 377a12915eebf50cb5b065858a711b7ddfa97d01 && git checkout -B drift-repro df8bb40a032c016276e0b16c5edbd90cad086c9d && git merge --no-ff 377a12915eebf50cb5b065858a711b7ddfa97d01
node scripts/docs-audit/affected-docs.mjs --json df8bb40a032c016276e0b16c5edbd90cad086c9d
|
席位核对 · ACCEPT ·
|
Part of #24835
Clause-②: no
#24835 remains open after this PR: it carries the log-level half only. The delivery loss itself has a measured root cause in
packages/drivers/driver-sql(with the event built inpackages/objectql), which isdomain:engine's code, so it is reported on the card as a fork and not patched here.What this PR changes
When the
sys_http_deliverywrite for a webhook event failed,AutoEnqueuer.reportWriteFailurelogged one bareWARN [webhook-auto-enqueuer] enqueue failedper event. Under AGENTS.md "Degradation log levels" that is a durability loss, not a functional one. The record write that produced the event already answered 200, the webhook still reads active, and no delivery row exists, so the event is never sent, retried or redeliverable, and nothing else looks wrong.error. The line names the webhook, says the event is lost and why nothing else shows it, and says where the cause is (the record's meta) and what to restore.writeFailing, the enqueue twin ofdroppedForSecret). Each further lost event for that webhook is kept atdebugwith itseventId..then(recordedWrite, reportWriteFailure)).errorstill gets the line atwarn(the file's existing fallback). The parked-subscription branch is unchanged.Measured root cause of the PostgreSQL loss (not changed here)
I reproduced it with the real
ObjectQLengine and the realSqlDriveron a throwaway local PostgreSQL 16.15 cluster, and in-memory SQLite. The realAutoEnqueuerwas wired the production way to the realMessagingServiceandSqlHttpOutbox. I ran one insert and one by-id update, then readsys_http_delivery:data.record.updatedpayload41646a5788payload.changes.updated_atis a knexRawWARN … enqueue failed … Converting circular structure to JSON … 'Timeout'9a8a7f5103, 2 updatesRawERRORnaming the webhook, then oneDEBUGfor the second loss9a8a7f5103, driver handed a copy of the patch, 2 updatesThe chain, at
41646a5788:ObjectQL.updatepasseshookContext.input.datatodriver.update(packages/objectql/src/engine.ts:18094). It then publishes the same object asDataEvent.changes(:18530).SqlDriver.updatebuildsformattedfromformatInput(packages/drivers/driver-sql/src/sql-driver.ts:8767). For a patch that needs no coercion,formatInputreturns the caller's object itself (:20336,:20526), and so doesapplyWriteColumnMapfor a managed object. Thenformatted.updated_at = this.updatedAtStamp()(:8778) writes into the caller's patch.updatedAtStamp()isknex.fn.now()(:8579), aRawthat holds the knex client, its pool and a NodeTimeout. On SQLite it is an ISO string (:8578). That string is just as foreign to the caller's patch, but it serialises.projectRecordBodiescopieschangesshallowly, andsql-http-outbox.ts:192JSON.stringifythrows on it.rotatedUpdateByIdhas the same in-place stamp (:10207).updateManyalready builds a fresh object instead (:10152). The fork, the candidate fix sites and the evidence are on the card.Not added: a serialisation guard in
sql-http-outbox.tsReading of the dispatch's decision frame: it does not belong beside the root-cause fix.
errorline in this PR makes that refusal loud, with its cause in the meta.Tests
Run on head
377a12915e(after mergingorigin/main, which brought the step-7c rewrite of the same file):pnpm --filter @objectstack/plugin-webhooks exec vitest run --maxWorkers=2:Test Files 21 passed (21),Tests 226 passed (226).pnpm --filter @objectstack/plugin-webhooks typecheck: exit 0 (tsc --noEmit,tsconfig.scripts.json,check:test-typecheck: OK).pnpm exec turbo run build --filter=@objectstack/plugin-webhooks...:17 successful, 17 total.New pins, in
src/auto-enqueuer.test.tsunder "a delivery that could not be recorded is reported at error":errornaming the webhook andsys_http_delivery, with theeventIdand the cause in meta, and later losses go todebug, neverwarn;error;They assert level, count and the named subjects, not prose.
Ablation (one leg, the changed behaviour)
node scripts/ablation-replace.mjsreplacedthis.writeFailing.add(sub.id);with the old barewarnandreturn. The anchor went from 1 hit to 0, and the blob from81ae0a56ccfato17c85086162a. Thenvitest run src/auto-enqueuer.test.tsgaveTests 4 failed | 40 passed (44): exactly the four new pins. The restore wasgit checkout HEAD. The restored blob is81ae0a56ccfa, equal to the HEAD blob, andgit diff HEADis empty. This ran on head377a12915e; the same leg on the pre-merge head9a8a7f5103read the same 4 failed and 40 passed. The subject is imported from source (./auto-enqueuer.js), so nodist/was involved.Gates
The local subset derived by
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackat377a12915ehas 70 families. Each exit was captured before any pipe.check:engine-double-contract,check:nul-bytes,check:doc-authoring,check:issue-citations,check:cross-package-test-inputs,check:test-source-alias,check:type-check-coverage,check:published-files,check:dts-closure,check:lean-entry-closure,check:adr-0087-registration --base origin/main,check:changeset-no-majorandcheck:empty-changeset.check:dual-build-cjs-loadsneeds every package'sdist/.check:i18nneeds the CLI build closure. Neither reads anything this diff changes. CI'sLint & Repo Gatesruns both.check:type-check-debt(--re-measure) rebuilds the whole monorepo under the shared lock. CI'sLint & Repo Gatesruns it.--ranreconciliation:70 derived, 67 run, 2 NOT-MEASURED, 1 UNRUN(the UNRUN is the declaredcheck:type-check-debt). The full-batteryTemporal Conformance (live PG + MySQL)job does not run on a PR. CI check-runs were not concluded when this body was written; the report on #24835 cites them.Acceptance notes
changesand ofhookContext.input.dataafter the driver call. On PostgreSQL and MySQL that is a knexRaw; on SQLite it is a driver-stampedupdated_atthe caller never wrote. I measured only the webhook reader here.error, with no say-once. I left it as it was. It is outside this card's defect.pnpm check:durability-log-levelcannot see this seam, because it judgestry/catchblocks and this is a promise rejection handler. I did not extend it (new gates default to no).Generated by Claude Code