Skip to content

Commit 6085a0d

Browse files
hotlongCopilot
andcommitted
fix(objectql): thread transaction through beforeUpdate/beforeDelete hooks
Prior to this fix, calling engine.update or engine.delete inside an engine.transaction() block would deadlock for ~60s on SQLite (knex pool=1) before completing. Root cause: the built-in sys_fetch_previous_update / sys_fetch_previous_delete hooks and the audit captureBefore hook both issue a findOne to snapshot the row, but neither carries the active transaction through. On a single- connection driver this read waits the full acquireConnectionTimeout for a fresh connection that never comes (the outer transaction owns the only one), then eventually proceeds when the trx commits. Concrete impact: convertLead (4 ops in a single transaction) took 60.1s end-to-end even though the actual SQL took <50ms. Fix: * sys_fetch_previous_update / delete hooks (packages/objectql/ src/plugin.ts) now pass context.isSystem + the open trx when hookCtx.transaction is set. * Audit captureBefore (packages/plugins/plugin-audit/src/audit- writers.ts) now prefers ctx.ql.findOne with isSystem + trx and only falls back to api.sudo() when no engine ref is available. Verified: 3 sequential convertLead calls now complete in 77ms-116ms (was 60.1s each). All 232 objectql tests, 55 plugin-security tests, and 215 metadata tests still pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6af96d1 commit 6085a0d

3 files changed

Lines changed: 32 additions & 7 deletions

File tree

packages/objectql/src/plugin.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,13 @@ export class ObjectQLPlugin implements Plugin {
400400
if (hookCtx.input?.id && !hookCtx.previous) {
401401
try {
402402
const existing = await this.ql!.findOne(hookCtx.object, {
403-
where: { id: hookCtx.input.id }
403+
where: { id: hookCtx.input.id },
404+
context: {
405+
roles: [],
406+
permissions: [],
407+
isSystem: true,
408+
...(hookCtx.transaction ? { transaction: hookCtx.transaction } : {}),
409+
} as any,
404410
});
405411
if (existing) hookCtx.previous = existing;
406412
} catch (_e) {
@@ -419,7 +425,13 @@ export class ObjectQLPlugin implements Plugin {
419425
if (hookCtx.input?.id && !hookCtx.previous) {
420426
try {
421427
const existing = await this.ql!.findOne(hookCtx.object, {
422-
where: { id: hookCtx.input.id }
428+
where: { id: hookCtx.input.id },
429+
context: {
430+
roles: [],
431+
permissions: [],
432+
isSystem: true,
433+
...(hookCtx.transaction ? { transaction: hookCtx.transaction } : {}),
434+
} as any,
423435
});
424436
if (existing) hookCtx.previous = existing;
425437
} catch (_e) {

packages/objectql/src/protocol.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,7 +1101,6 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
11011101
const runConversion = async (trxCtx: any) => {
11021102
const opCtx = trxCtx ?? ctx;
11031103
const trxCtxOpt = opCtx !== undefined ? { context: opCtx } : undefined;
1104-
console.log('[convertLead] entered runConversion, trx present:', !!opCtx?.transaction);
11051104

11061105
// 1) Account
11071106
let account: any;
@@ -1125,7 +1124,6 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
11251124
if (lead.address) accountPayload.billing_address = lead.address;
11261125
if (lead.owner) accountPayload.owner = lead.owner;
11271126
account = await this.engine.insert('account', accountPayload, trxCtxOpt as any);
1128-
console.log('[convertLead] account inserted');
11291127
}
11301128

11311129
// 2) Contact
@@ -1152,7 +1150,6 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
11521150
if (lead.owner) contactPayload.owner = lead.owner;
11531151
if (account?.id) contactPayload.account = account.id;
11541152
contact = await this.engine.insert('contact', contactPayload, trxCtxOpt as any);
1155-
console.log('[convertLead] contact inserted');
11561153
}
11571154

11581155
// 3) Opportunity (optional)

packages/plugins/plugin-audit/src/audit-writers.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,25 @@ export function installAuditWriters(engine: any, packageId = 'com.objectstack.au
120120
if (SKIP_OBJECTS.has(ctx.object)) return;
121121
const id = (ctx.input as any)?.id;
122122
if (!id) return; // bulk update/delete — too costly to snapshot every row here
123-
const api: any = (ctx as any).api;
124-
if (!api?.sudo) return;
125123
try {
124+
// Use the engine directly (not api.sudo) so we can thread the
125+
// active transaction through. On drivers with single-connection
126+
// pools (e.g. SQLite via knex) a sudo() findOne that does NOT
127+
// carry the open transaction will deadlock for the full
128+
// acquireConnectionTimeout (~60s) because the outer transaction
129+
// holds the only connection.
130+
const trx = (ctx as any).transaction;
131+
const ql = (ctx as any).ql ?? (ctx as any).api?.engine;
132+
if (ql?.findOne) {
133+
const prev = await ql.findOne(ctx.object, {
134+
where: { id },
135+
context: { isSystem: true, ...(trx ? { transaction: trx } : {}) },
136+
});
137+
if (prev) (ctx as any).__previous = prev;
138+
return;
139+
}
140+
const api: any = (ctx as any).api;
141+
if (!api?.sudo) return;
126142
const prev = await api.sudo().object(ctx.object).findOne({ where: { id } });
127143
if (prev) (ctx as any).__previous = prev;
128144
} catch {

0 commit comments

Comments
 (0)