Skip to content

Commit 74b9042

Browse files
matt-aitkenclaude
andcommitted
fix(rbac,testcontainers): make e2e healthcheck-require-plugins actually work
Three small fixes uncovered by running the e2e test locally: - internal-packages/rbac/src/index.ts: attach a no-op .catch() to LazyController._init in the constructor. load() runs eagerly but its result is only awaited on the first method call. When load() rejects (REQUIRE_PLUGINS=1 + plugin missing), Node flags the unawaited rejection as unhandledRejection — newer Node versions kill the process before any consumer can await _init and convert the throw into a 500. The .catch() marks the rejection as handled at the global level; the actual error still re-throws when c() awaits _init. - internal-packages/testcontainers/src/webapp.ts: when requirePlugins is set, explicitly override RBAC_FORCE_FALLBACK="0" so a local apps/webapp/.env that sets it to "1" doesn't short-circuit the loader past the REQUIRE_PLUGINS check. Without this, the test passes in CI (no .env file) but fails locally with no obvious error. - internal-packages/testcontainers/src/webapp.ts: waitForHealthcheck now takes an `acceptAnyResponse` flag. When requirePlugins is set, /healthcheck is *expected* to return 500 (the whole point of the test), so the bootstrap waiter must accept any HTTP response rather than only 200 — otherwise startTestServer times out before the test can assert. apps/webapp/test/healthcheck-require-plugins.e2e.test.ts: tidied up the locally-linked-skip placeholder. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ee6bce6 commit 74b9042

3 files changed

Lines changed: 52 additions & 5 deletions

File tree

apps/webapp/test/healthcheck-require-plugins.e2e.test.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,32 @@
1010
* env can differ per case. Slow but isolated, matching api-auth.e2e.test.ts.
1111
*
1212
* Requires a pre-built webapp: pnpm run build --filter webapp
13+
*
14+
* The REQUIRE_PLUGINS=1 case relies on the plugin NOT being resolvable
15+
* from the spawned webapp. CI satisfies this because the plugin isn't in
16+
* pnpm-lock.yaml. Local devs who ran `pnpm dev:link-webapp` have the
17+
* plugin symlinked into apps/webapp/node_modules — that case is detected
18+
* and skipped below.
1319
*/
20+
import { existsSync } from "node:fs";
21+
import { resolve } from "node:path";
1422
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
1523
import type { TestServer } from "@internal/testcontainers/webapp";
1624
import { startTestServer } from "@internal/testcontainers/webapp";
1725

26+
const LINKED_PLUGIN_PATH = resolve(
27+
__dirname,
28+
"..",
29+
"node_modules",
30+
"@triggerdotdev",
31+
"plugins"
32+
);
33+
const pluginLocallyLinked = existsSync(LINKED_PLUGIN_PATH);
34+
1835
vi.setConfig({ testTimeout: 180_000 });
1936

2037
describe("/healthcheck with REQUIRE_PLUGINS", () => {
21-
describe("REQUIRE_PLUGINS=1 + plugin missing", () => {
38+
describe.skipIf(pluginLocallyLinked)("REQUIRE_PLUGINS=1 + plugin missing", () => {
2239
let server: TestServer;
2340

2441
beforeAll(async () => {
@@ -42,6 +59,17 @@ describe("/healthcheck with REQUIRE_PLUGINS", () => {
4259
});
4360
});
4461

62+
// Surface the skip in dev so it doesn't go unnoticed. CI hits the real test above.
63+
describe.runIf(pluginLocallyLinked)(
64+
"REQUIRE_PLUGINS=1 + plugin LOCALLY LINKED (cross-repo dev setup)",
65+
() => {
66+
it.skip(
67+
`skipped because ${LINKED_PLUGIN_PATH} exists — plugin would load successfully. Run \`pnpm dev:unlink-webapp\` to exercise this case locally; CI runs it without the link.`,
68+
() => {}
69+
);
70+
}
71+
);
72+
4573
describe("REQUIRE_PLUGINS unset + plugin missing", () => {
4674
let server: TestServer;
4775

internal-packages/rbac/src/index.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@ class LazyController implements RoleBaseAccessController {
5353

5454
constructor(prisma: RbacPrismaInput, options?: RbacCreateOptions) {
5555
this._init = this.load(prisma, options);
56+
// load() runs eagerly but the result is awaited lazily on first method
57+
// call. If load() rejects (e.g. REQUIRE_PLUGINS=1 + plugin missing) and
58+
// nothing awaits _init before Node ticks past, the rejection surfaces
59+
// as unhandledRejection and kills the process. Attach a no-op .catch
60+
// so Node sees the rejection as handled; the error is re-thrown when
61+
// any consumer awaits this._init via c().
62+
this._init.catch(() => {});
5663
}
5764

5865
private async load(

internal-packages/testcontainers/src/webapp.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,16 @@ async function findFreePort(): Promise<number> {
2020
});
2121
}
2222

23-
async function waitForHealthcheck(url: string, timeoutMs = 60000): Promise<void> {
23+
async function waitForHealthcheck(
24+
url: string,
25+
opts: { timeoutMs?: number; acceptAnyResponse?: boolean } = {}
26+
): Promise<void> {
27+
const { timeoutMs = 60000, acceptAnyResponse = false } = opts;
2428
const deadline = Date.now() + timeoutMs;
2529
while (Date.now() < deadline) {
2630
try {
2731
const res = await fetch(url);
28-
if (res.ok) return;
32+
if (acceptAnyResponse || res.ok) return;
2933
} catch {}
3034
await new Promise((r) => setTimeout(r, 500));
3135
}
@@ -122,7 +126,10 @@ export async function startWebapp(
122126
// plugin is installed in the local node_modules. Set to "0" /
123127
// undefined to spawn a webapp that loads any installed plugin.
124128
...(forceRbacFallback ? { RBAC_FORCE_FALLBACK: "1" } : {}),
125-
...(requirePlugins ? { REQUIRE_PLUGINS: "1" } : {}),
129+
// When requirePlugins is set, explicitly override RBAC_FORCE_FALLBACK
130+
// to "0" so a local apps/webapp/.env that sets it to "1" doesn't
131+
// short-circuit the loader past the REQUIRE_PLUGINS check.
132+
...(requirePlugins ? { REQUIRE_PLUGINS: "1", RBAC_FORCE_FALLBACK: "0" } : {}),
126133
NODE_PATH: nodePath,
127134
},
128135
stdio: ["ignore", "pipe", "pipe"],
@@ -158,7 +165,12 @@ export async function startWebapp(
158165

159166
try {
160167
if (spawnError) throw spawnError;
161-
await waitForHealthcheck(`${baseUrl}/healthcheck`);
168+
// When requirePlugins is set, /healthcheck is expected to respond with
169+
// 500 (the whole point of those tests is to verify that). Accept any
170+
// response as "the webapp is up" — the test then asserts the status.
171+
await waitForHealthcheck(`${baseUrl}/healthcheck`, {
172+
acceptAnyResponse: requirePlugins,
173+
});
162174
if (spawnError) throw spawnError;
163175
} catch (err) {
164176
proc.kill("SIGTERM");

0 commit comments

Comments
 (0)