Skip to content

Commit 15d5a45

Browse files
marcusbellamyshaw-cellclaudeascorbic
authored
fix(core): defer afterDelete and afterUnpublish hooks so Workers doesn't cancel them (#1588)
runAfterDeleteHooks and runAfterUnpublishHooks dispatched plugin hooks fire-and-forget. On Cloudflare Workers a promise not handed to waitUntil is canceled when the response returns, so plugin cleanup work (R2 deletes, search-index removal) is killed mid-flight -- and a half-completed plugin-storage write can wedge the isolate and hang later admin requests. Wrap both dispatchers in after(), exactly as runAfterSaveHooks and runAfterPublishHooks already do. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Matt Kane <mkane@cloudflare.com>
1 parent b828198 commit 15d5a45

3 files changed

Lines changed: 181 additions & 17 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"emdash": patch
3+
---
4+
5+
Fixes plugin `content:afterDelete` and `content:afterUnpublish` hooks being dropped on Cloudflare Workers. Cleanup work in these hooks — removing uploaded files, clearing search indexes — now runs to completion after content is deleted or unpublished, instead of being canceled the moment the response is sent. Previously this could also leave plugin storage in a wedged state that hung later admin requests.

packages/core/src/emdash-runtime.ts

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2965,7 +2965,7 @@ export class EmDashRuntime {
29652965
await this.refreshContentUsageAfterSuccessfulWrite(collection, [result.data.id]);
29662966
}
29672967

2968-
// Run afterDelete hooks (fire-and-forget)
2968+
// Run afterDelete hooks (deferred past the response via after())
29692969
if (result.success) {
29702970
this.runAfterDeleteHooks(id, collection, false);
29712971
}
@@ -3052,7 +3052,7 @@ export class EmDashRuntime {
30523052
await this.refreshContentUsageAfterSuccessfulWrite(collection, [result.data.item.id]);
30533053
}
30543054

3055-
// Run afterUnpublish hooks (fire-and-forget)
3055+
// Run afterUnpublish hooks (deferred past the response via after())
30563056
if (result.success && result.data) {
30573057
this.runAfterUnpublishHooks(contentItemToRecord(result.data.item), collection);
30583058
}
@@ -3746,24 +3746,34 @@ export class EmDashRuntime {
37463746
}
37473747

37483748
private runAfterDeleteHooks(id: string, collection: string, permanent: boolean): void {
3749-
// Trusted plugins
3750-
if (this.hooks.hasHooks("content:afterDelete")) {
3751-
this.hooks
3752-
.runContentAfterDelete(id, collection, permanent)
3753-
.catch((err) => console.error("EmDash afterDelete hook error:", err));
3754-
}
3749+
after(async () => {
3750+
// Trusted plugins
3751+
if (this.hooks.hasHooks("content:afterDelete")) {
3752+
try {
3753+
await this.hooks.runContentAfterDelete(id, collection, permanent);
3754+
} catch (err) {
3755+
console.error("EmDash afterDelete hook error:", err);
3756+
}
3757+
}
37553758

3756-
// Sandboxed plugins
3757-
for (const [pluginKey, plugin] of this.sandboxedPlugins) {
3758-
const [pluginId] = pluginKey.split(":");
3759-
if (!pluginId || !this.isPluginEnabled(pluginId)) continue;
3759+
// Sandboxed plugins
3760+
const tasks: Promise<void>[] = [];
3761+
for (const [pluginKey, plugin] of this.sandboxedPlugins) {
3762+
const [pluginId] = pluginKey.split(":");
3763+
if (!pluginId || !this.isPluginEnabled(pluginId)) continue;
37603764

3761-
plugin
3762-
.invokeHook("content:afterDelete", { id, collection, permanent })
3763-
.catch((err) =>
3764-
console.error(`EmDash: Sandboxed plugin ${pluginId} afterDelete error:`, err),
3765+
tasks.push(
3766+
(async () => {
3767+
try {
3768+
await plugin.invokeHook("content:afterDelete", { id, collection, permanent });
3769+
} catch (err) {
3770+
console.error(`EmDash: Sandboxed plugin ${pluginId} afterDelete error:`, err);
3771+
}
3772+
})(),
37653773
);
3766-
}
3774+
}
3775+
await Promise.allSettled(tasks);
3776+
});
37673777
}
37683778

37693779
private runDeferredContentHook(
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
/**
2+
* afterDelete / afterUnpublish hooks must be deferred through `after()`.
3+
*
4+
* On Cloudflare Workers, a promise that isn't handed to the host's
5+
* lifetime extender (`waitUntil`, which `after()` wraps) is canceled the
6+
* moment the HTTP response is returned. `afterSave` and `afterPublish`
7+
* already defer their hook dispatch through `after()`; `afterDelete` and
8+
* `afterUnpublish` historically did not — they fired the hook promise and
9+
* forgot it. A plugin doing real I/O in those hooks (storage cleanup,
10+
* search-index removal) would have that work killed mid-flight, which on
11+
* Workers can wedge the plugin-storage backend and hang every subsequent
12+
* request in the isolate.
13+
*
14+
* These tests pin the contract: deleting or unpublishing content schedules
15+
* the hook work via `after()` instead of running it inline-and-abandoned.
16+
*/
17+
18+
import { randomUUID } from "node:crypto";
19+
20+
import Database from "better-sqlite3";
21+
import { SqliteDialect } from "kysely";
22+
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
23+
24+
// Capture-only stub for `after()`: record the deferred task without running
25+
// it, so a test can assert the work was scheduled (not abandoned) and then
26+
// flush it deliberately. Mirrors how Workers holds the promise via waitUntil.
27+
const { deferred } = vi.hoisted(() => ({ deferred: [] as Array<() => void | Promise<void>> }));
28+
vi.mock("../../../src/after.js", () => ({
29+
after: (fn: () => void | Promise<void>) => {
30+
deferred.push(fn);
31+
},
32+
}));
33+
34+
import { ContentRepository } from "../../../src/database/repositories/content.js";
35+
import { EmDashRuntime } from "../../../src/emdash-runtime.js";
36+
import type { RuntimeDependencies } from "../../../src/emdash-runtime.js";
37+
import { definePlugin } from "../../../src/plugins/define-plugin.js";
38+
import type {
39+
ContentAfterDeleteHandler,
40+
ContentAfterUnpublishHandler,
41+
} from "../../../src/plugins/types.js";
42+
import { SchemaRegistry } from "../../../src/schema/registry.js";
43+
44+
const afterDeleteHandler = vi.fn<ContentAfterDeleteHandler>(async () => {});
45+
const afterUnpublishHandler = vi.fn<ContentAfterUnpublishHandler>(async () => {});
46+
47+
function createDeps(sqlite: Database.Database): RuntimeDependencies {
48+
return {
49+
config: {
50+
database: {
51+
entrypoint: `test-after-hooks-${randomUUID()}`,
52+
config: {},
53+
type: "sqlite",
54+
},
55+
},
56+
plugins: [
57+
definePlugin({
58+
id: "lifecycle-watcher",
59+
version: "1.0.0",
60+
// afterDelete / afterUnpublish are read-only notifications: they
61+
// require content:read to register.
62+
capabilities: ["content:read"],
63+
hooks: {
64+
"content:afterDelete": { handler: afterDeleteHandler },
65+
"content:afterUnpublish": { handler: afterUnpublishHandler },
66+
},
67+
}),
68+
],
69+
createDialect: () => new SqliteDialect({ database: sqlite }),
70+
createStorage: null,
71+
sandboxEnabled: false,
72+
sandboxedPluginEntries: [],
73+
createSandboxRunner: null,
74+
};
75+
}
76+
77+
/** Let any synchronous fire-and-forget microtasks settle. */
78+
const drainMicrotasks = () => new Promise((resolve) => setTimeout(resolve, 0));
79+
80+
/** Run everything the runtime handed to `after()`, like waitUntil would. */
81+
async function flushDeferred(): Promise<void> {
82+
const tasks = deferred.splice(0);
83+
for (const task of tasks) await task();
84+
}
85+
86+
describe("runtime defers lifecycle hooks through after()", () => {
87+
let runtime: EmDashRuntime;
88+
let repo: ContentRepository;
89+
90+
beforeEach(async () => {
91+
deferred.length = 0;
92+
afterDeleteHandler.mockClear();
93+
afterUnpublishHandler.mockClear();
94+
95+
const sqlite = new Database(":memory:");
96+
runtime = await EmDashRuntime.create(createDeps(sqlite));
97+
98+
const registry = new SchemaRegistry(runtime.db);
99+
await registry.createCollection({ slug: "post", label: "Posts", labelSingular: "Post" });
100+
await registry.createField("post", { slug: "title", label: "Title", type: "string" });
101+
102+
repo = new ContentRepository(runtime.db);
103+
104+
// Drop any boot-time deferred work so each test observes only its own.
105+
deferred.length = 0;
106+
});
107+
108+
afterEach(async () => {
109+
await runtime.stopCron();
110+
});
111+
112+
it("schedules the afterDelete hook via after() on soft delete", async () => {
113+
const item = await repo.create({ type: "post", data: { title: "Doomed" } });
114+
115+
deferred.length = 0;
116+
const result = await runtime.handleContentDelete("post", item.id);
117+
expect(result.success).toBe(true);
118+
119+
// If the dispatch were fire-and-forget, the handler would have run by
120+
// now. Correct behavior defers it, so it has NOT run yet...
121+
await drainMicrotasks();
122+
expect(afterDeleteHandler).not.toHaveBeenCalled();
123+
expect(deferred.length).toBeGreaterThan(0);
124+
125+
// ...and running what was handed to after() (as waitUntil would) fires it.
126+
await flushDeferred();
127+
expect(afterDeleteHandler).toHaveBeenCalledTimes(1);
128+
});
129+
130+
it("schedules the afterUnpublish hook via after() on unpublish", async () => {
131+
const item = await repo.create({ type: "post", data: { title: "Live then gone" } });
132+
const published = await runtime.handleContentPublish("post", item.id);
133+
expect(published.success).toBe(true);
134+
135+
// Clear the afterPublish deferral so we observe only the unpublish.
136+
deferred.length = 0;
137+
afterUnpublishHandler.mockClear();
138+
139+
const result = await runtime.handleContentUnpublish("post", item.id);
140+
expect(result.success).toBe(true);
141+
142+
await drainMicrotasks();
143+
expect(afterUnpublishHandler).not.toHaveBeenCalled();
144+
expect(deferred.length).toBeGreaterThan(0);
145+
146+
await flushDeferred();
147+
expect(afterUnpublishHandler).toHaveBeenCalledTimes(1);
148+
});
149+
});

0 commit comments

Comments
 (0)