Skip to content

Commit

Permalink
Ensure Miniflare instances disposed
Browse files Browse the repository at this point in the history
  • Loading branch information
mrbbot committed Aug 17, 2023
1 parent 38e067d commit 62c842e
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 2 deletions.
1 change: 1 addition & 0 deletions ava.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const rewritePaths = Object.fromEntries(
export default {
files: ["packages/*/test/**/*.spec.ts"],
nodeArguments: ["--no-warnings", "--experimental-vm-modules"],
require: ["./packages/miniflare/test/setup.mjs"],
workerThreads: inspector.url() === undefined,
typescript: {
compile: false,
Expand Down
22 changes: 22 additions & 0 deletions packages/miniflare/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,15 @@ function safeReadableStreamFrom(iterable: AsyncIterable<Uint8Array>) {
);
}

// Maps `Miniflare` instances to stack traces for thier construction. Used to identify un-`dispose()`d instances.
let maybeInstanceRegistry:
| Map<Miniflare, string /* constructionStack */>
| undefined;
/** @internal */
export function _initialiseInstanceRegistry() {
return (maybeInstanceRegistry = new Map());
}

export class Miniflare {
readonly #gatewayFactories: PluginGatewayFactories;
readonly #routers: PluginRouters;
Expand Down Expand Up @@ -496,6 +505,15 @@ export class Miniflare {
const [sharedOpts, workerOpts] = validateOptions(opts);
this.#sharedOpts = sharedOpts;
this.#workerOpts = workerOpts;

// Add to registry after initial options validation, before any servers/
// child processes are started
if (maybeInstanceRegistry !== undefined) {
const object = { name: "Miniflare", stack: "" };
Error.captureStackTrace(object, Miniflare);
maybeInstanceRegistry.set(this, object.stack);
}

this.#log = this.#sharedOpts.core.log ?? new NoOpLog();
this.#timers = this.#sharedOpts.core.timers ?? defaultTimers;
this.#host = this.#sharedOpts.core.host ?? "127.0.0.1";
Expand Down Expand Up @@ -1247,6 +1265,10 @@ export class Miniflare {
await this.#stopLoopbackServer();
// `rm -rf ${#tmpPath}`, this won't throw if `#tmpPath` doesn't exist
await fs.promises.rm(this.#tmpPath, { force: true, recursive: true });

// Remove from instance registry as last step in `finally`, to make sure
// all dispose steps complete
maybeInstanceRegistry?.delete(this);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion packages/miniflare/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ test("Miniflare: accepts https requests", async (t) => {
t.assert(log.logs[0][1].startsWith("Ready on https://"));
});

test("Miniflare: Manually triggered scheduled events", async (t) => {
test("Miniflare: manually triggered scheduled events", async (t) => {
const log = new TestLog(t);

const mf = new Miniflare({
Expand All @@ -545,6 +545,7 @@ test("Miniflare: Manually triggered scheduled events", async (t) => {
}
}`,
});
t.teardown(() => mf.dispose());

let res = await mf.dispatchFetch("http://localhost");
t.is(await res.text(), "false");
Expand Down
1 change: 1 addition & 0 deletions packages/miniflare/test/plugins/core/errors/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ addEventListener("fetch", (event) => {
},
],
});
t.teardown(() => mf.dispose());

// Check service-workers source mapped
let error = await t.throwsAsync(mf.dispatchFetch("http://localhost"), {
Expand Down
11 changes: 11 additions & 0 deletions packages/miniflare/test/plugins/core/proxy/client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ test("ProxyClient: supports service bindings with WebSockets", async (t) => {
},
},
});
t.teardown(() => mf.dispose());

const { CUSTOM } = await mf.getBindings<{
CUSTOM: ReplaceWorkersTypes<Fetcher>;
}>();
Expand All @@ -53,6 +55,8 @@ test("ProxyClient: supports service bindings with WebSockets", async (t) => {

test("ProxyClient: supports serialising multiple ReadableStreams, Blobs and Files", async (t) => {
const mf = new Miniflare({ script: nullScript });
t.teardown(() => mf.dispose());

const client = await mf._getProxyClient();
const IDENTITY = client.env.IDENTITY as {
asyncIdentity<Args extends any[]>(...args: Args): Promise<Args>;
Expand Down Expand Up @@ -130,6 +134,8 @@ test("ProxyClient: poisons dependent proxies after setOptions()/dispose()", asyn
});
test("ProxyClient: logging proxies provides useful information", async (t) => {
const mf = new Miniflare({ script: nullScript });
t.teardown(() => mf.dispose());

const caches = await mf.getCaches();
const inspectOpts: util.InspectOptions = { colors: false };
t.is(
Expand Down Expand Up @@ -160,6 +166,7 @@ test("ProxyClient: stack traces don't include internal implementation", async (t
// https://developers.cloudflare.com/workers/configuration/compatibility-dates/#do-not-throw-from-async-functions
compatibilityFlags: ["capture_async_api_throws"],
});
t.teardown(() => mf.dispose());

const ns = await mf.getDurableObjectNamespace("OBJECT");
const caches = await mf.getCaches();
Expand Down Expand Up @@ -189,6 +196,8 @@ test("ProxyClient: stack traces don't include internal implementation", async (t
});
test("ProxyClient: can access ReadableStream property multiple times", async (t) => {
const mf = new Miniflare({ script: nullScript, r2Buckets: ["BUCKET"] });
t.teardown(() => mf.dispose());

const bucket = await mf.getR2Bucket("BUCKET");
await bucket.put("key", "value");
const objectBody = await bucket.get("key");
Expand All @@ -198,6 +207,8 @@ test("ProxyClient: can access ReadableStream property multiple times", async (t)
});
test("ProxyClient: returns empty ReadableStream synchronously", async (t) => {
const mf = new Miniflare({ script: nullScript, r2Buckets: ["BUCKET"] });
t.teardown(() => mf.dispose());

const bucket = await mf.getR2Bucket("BUCKET");
await bucket.put("key", "");
const objectBody = await bucket.get("key");
Expand Down
11 changes: 11 additions & 0 deletions packages/miniflare/test/plugins/queues/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ test("flushes partial and full batches", async (t) => {
},
],
});
t.teardown(() => mf.dispose());

async function send(message: unknown) {
await mf.dispatchFetch("http://localhost/send", {
method: "POST",
Expand Down Expand Up @@ -255,6 +257,7 @@ test("sends all structured cloneable types", async (t) => {
},
],
});
t.teardown(() => mf.dispose());

await mf.dispatchFetch("http://localhost");
timers.timestamp += 1000;
Expand Down Expand Up @@ -326,6 +329,8 @@ test("retries messages", async (t) => {
}
}`,
});
t.teardown(() => mf.dispose());

async function sendBatch(...messages: string[]) {
await mf.dispatchFetch("http://localhost", {
method: "POST",
Expand Down Expand Up @@ -546,6 +551,8 @@ test("moves to dead letter queue", async (t) => {
}
}`,
});
t.teardown(() => mf.dispose());

async function sendBatch(...messages: string[]) {
await mf.dispatchFetch("http://localhost", {
method: "POST",
Expand Down Expand Up @@ -648,6 +655,8 @@ test("operations permit strange queue names", async (t) => {
}
}`,
});
t.teardown(() => mf.dispose());

await mf.dispatchFetch("http://localhost");
timers.timestamp += 1000;
await timers.waitForTasks();
Expand Down Expand Up @@ -718,6 +727,8 @@ test("supports message contentTypes", async (t) => {
},
};`,
});
t.teardown(() => mf.dispose());

const res = await mf.dispatchFetch("http://localhost");
await res.arrayBuffer(); // (drain)
timers.timestamp += 1000;
Expand Down
23 changes: 23 additions & 0 deletions packages/miniflare/test/setup.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { _initialiseInstanceRegistry } from "miniflare";

const registry = _initialiseInstanceRegistry();
const bigSeparator = "=".repeat(80);
const separator = "-".repeat(80);

// `process.on("exit")` is more like `worker_thread.on(`exit`)` here. It will
// be called once AVA's finished running tests and `after` hooks. Note we can't
// use an `after` hook here, as that would run before `miniflareTest`'s
// `after` hooks to dispose their `Miniflare` instances.
process.on("exit", () => {
if (registry.size === 0) return;

// If there are Miniflare instances that weren't disposed, throw
const s = registry.size === 1 ? "" : "s";
const was = registry.size === 1 ? "was" : "were";
const message = `Found ${registry.size} Miniflare instance${s} that ${was} not dispose()d`;
const stacks = Array.from(registry.values()).join(`\n${separator}\n`);
console.log(
[bigSeparator, message, separator, stacks, bigSeparator].join("\n")
);
throw new Error(message);
});
2 changes: 1 addition & 1 deletion packages/miniflare/test/test-shared/miniflare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,6 @@ export function miniflareTest<
t.context.mf.setOptions({ ...userOpts, ...opts } as MiniflareOptions);
t.context.url = await t.context.mf.ready;
});
test.after((t) => t.context.mf.dispose());
test.after.always((t) => t.context.mf.dispose());
return test;
}

0 comments on commit 62c842e

Please sign in to comment.