From 5fdc1cf1eb88936812003c5140b296e0ee320416 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Emre=20Kabakc=C4=B1?= Date: Sun, 17 May 2026 01:28:20 +0100 Subject: [PATCH 1/2] fix(insert-event): guard against empty INSERT RETURNING MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `insertEvent` assumed `INSERT ... RETURNING` always yields a row, then dereferenced `result[0].id` to feed `upsertEmbedding`. When the result came back empty — exposed by the menu bar's no-auth sync round-trip completing for the first time — the call crashed with the cryptic `TypeError: Cannot read properties of undefined (reading 'id')` and the worker stream returned 500. Two defensive guards: 1. After the main INSERT path, treat `result[0]` as possibly undefined. Log connector/feed/origin context and throw a real error explaining the row wasn't persisted. Callers (worker streamContent) catch the throw and surface a useful message instead of the stack-trace blob. 2. In the conflict-update early-return path, `existingRows[0]` could also be undefined if the found row was deleted/tombstoned between `findCurrentEventByOrigin` and the subsequent reread (race). Fall through to the fresh-insert path instead of crashing. Root cause of the original empty-result is still under investigation — likely a PGlite quirk with the new `search_tsv tsvector GENERATED ALWAYS AS (...) STORED` column from PR #765, or a transient state during bootstrap-org/connector wire-up. Either way, the right behavior is to fail loud with context rather than crash on `undefined.id`. --- packages/server/src/utils/insert-event.ts | 42 +++++++++++++++++++++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/packages/server/src/utils/insert-event.ts b/packages/server/src/utils/insert-event.ts index 60aa89f0d..9b86222bc 100644 --- a/packages/server/src/utils/insert-event.ts +++ b/packages/server/src/utils/insert-event.ts @@ -234,9 +234,19 @@ export async function insertEvent( WHERE id = ${existing.id} LIMIT 1 `; - return existingRows[0] as InsertedEvent; + const existingRow = existingRows[0] as InsertedEvent | undefined; + if (existingRow) return existingRow; + // Race: the existing row was deleted/tombstoned between the + // findCurrentEventByOrigin lookup above and the SELECT here. Fall + // through into the INSERT path with `supersedesEventId` unset so we + // create a fresh row instead of crashing on `undefined.id`. + logger.warn( + { existingId: existing.id, originId: params.originId }, + '[insert-event] existing row vanished between find and reread — proceeding with fresh insert' + ); + } else { + supersedesEventId = existing.id; } - supersedesEventId = existing.id; } } @@ -299,7 +309,33 @@ export async function insertEvent( result = await insertWithClientId(null); } - const inserted = result[0] as InsertedEvent; + const inserted = result[0] as InsertedEvent | undefined; + if (!inserted) { + // INSERT ... RETURNING should always yield a row for a successful insert. + // An empty result means either (a) a BEFORE INSERT trigger silently + // RETURNed NULL (none in our schema today), (b) a PGlite quirk around + // GENERATED STORED columns failing without surfacing an error, or + // (c) postgres.js dropping the rows for an obscure reason. None of these + // should drop events on the floor — convert the cryptic `Cannot read + // properties of undefined (reading 'id')` into a real error with + // diagnostic context so we can root-cause when it next happens. + logger.error( + { + originId: params.originId, + connectionId: params.connectionId, + organizationId: params.organizationId, + semanticType: params.semanticType, + connectorKey: params.connectorKey, + feedKey: params.feedKey, + }, + '[insert-event] INSERT ... RETURNING returned no rows — event not persisted' + ); + throw new Error( + `insertEvent: INSERT RETURNING came back empty for ` + + `origin_id=${params.originId} connection_id=${params.connectionId}. ` + + `Row was not persisted; check server logs for diagnostic context.` + ); + } await upsertEmbedding(inserted.id, params.embedding, sql); return inserted; } From c8715e17310928e63bf34b93a0d07c75674f11a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Emre=20Kabakc=C4=B1?= Date: Sun, 17 May 2026 01:37:23 +0100 Subject: [PATCH 2/2] fix(insert-event): move embedding upsert after reread (CodeRabbit catch) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The conflict-update early-return path called upsertEmbedding(existing.id, ...) before the SELECT that confirms the row is still there. If the row got deleted/tombstoned in that window, upsertEmbedding would FK-fail with a confusing error instead of letting us fall through to a fresh insert. Reorder so the embedding upsert only fires when the reread confirms the event still exists. Fall-through behavior on the race is unchanged. The 3 other CodeRabbit comments on this PR were against code from PR #779 (now merged) — those will land as separate follow-ups since they're out of scope here. --- packages/server/src/utils/insert-event.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/server/src/utils/insert-event.ts b/packages/server/src/utils/insert-event.ts index 9b86222bc..d7950b1f1 100644 --- a/packages/server/src/utils/insert-event.ts +++ b/packages/server/src/utils/insert-event.ts @@ -227,7 +227,11 @@ export async function insertEvent( const existing = await findCurrentEventByOrigin(sql, params); if (existing) { if (isSemanticallyEqual(existing, params)) { - await upsertEmbedding(existing.id, params.embedding, sql); + // Reread before touching the embedding. If the row got + // deleted / tombstoned between findCurrentEventByOrigin and + // here, upsertEmbedding would FK-fail instead of letting us + // fall through cleanly to a fresh insert (CodeRabbit catch + // on PR #780). const existingRows = await sql` SELECT id, entity_ids, origin_id, title, semantic_type, created_at FROM events @@ -235,7 +239,10 @@ export async function insertEvent( LIMIT 1 `; const existingRow = existingRows[0] as InsertedEvent | undefined; - if (existingRow) return existingRow; + if (existingRow) { + await upsertEmbedding(existingRow.id, params.embedding, sql); + return existingRow; + } // Race: the existing row was deleted/tombstoned between the // findCurrentEventByOrigin lookup above and the SELECT here. Fall // through into the INSERT path with `supersedesEventId` unset so we