-
-
Notifications
You must be signed in to change notification settings - Fork 678
fix(collab): ship the relay image's runtime dependencies #1867
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | |||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,14 +8,40 @@ COPY packages/collab-core packages/collab-core | ||||||||||
| COPY workers/collab-node workers/collab-node | |||||||||||
| RUN npm run build -w geolibre-collab-node | |||||||||||
|
|
|||||||||||
| # Assemble the runtime tree under /runtime so the final stage is one COPY of a | |||||||||||
| # layout that is known to resolve. esbuild bundles everything except `ws`, and | |||||||||||
| # `ws` does not always land in /app/node_modules: the root lockfile carries two | |||||||||||
| # versions of it (8.21.0 hoisted for the app's transitive deps, ^8.21.3 for this | |||||||||||
| # worker), so npm installs this workspace's copy at the nested | |||||||||||
| # workers/collab-node/node_modules. Copying only /app/node_modules dropped it and | |||||||||||
| # the container exited at startup with ERR_MODULE_NOT_FOUND (GeoLibre#1866), so | |||||||||||
| # take the nested tree too, guarded because it disappears whenever a dependency | |||||||||||
| # bump lets npm hoist `ws` to the root instead. packages/collab-core comes along | |||||||||||
| # so the workspace symlink under node_modules resolves rather than dangling. The | |||||||||||
| # reinstall drops esbuild and the other dev dependencies now that the bundle is | |||||||||||
| # built, which is also what keeps the copied tree small. | |||||||||||
| RUN npm ci --omit=dev --workspace geolibre-collab-node --workspace @geolibre/collab-core \ | |||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor performance nit: this reinstalls with a fresh
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Measured this rather than guessing, and it does not hold up — leaving the second The literal suggestion, unscoped With those filters it works, but it is not faster. Both take 1s on an already-populated tree:
There is no second network round-trip to save: the first So it is the same speed, and |
|||||||||||
| && mkdir -p /runtime/workers/collab-node \ | |||||||||||
| && cp -a node_modules /runtime/node_modules \ | |||||||||||
| && cp -a packages /runtime/packages \ | |||||||||||
| && cp -a workers/collab-node/package.json workers/collab-node/dist /runtime/workers/collab-node/ \ | |||||||||||
| && if [ -d workers/collab-node/node_modules ]; then \ | |||||||||||
| cp -a workers/collab-node/node_modules /runtime/workers/collab-node/node_modules; \ | |||||||||||
| fi | |||||||||||
|
|
|||||||||||
| # Load the staged bundle from the path it will actually sit at, so a layout | |||||||||||
| # regression fails the build here instead of at container start. Importing it is | |||||||||||
| # side-effect-free: server.js only listens when it is process.argv[1]. | |||||||||||
| RUN node --input-type=module \ | |||||||||||
| -e "await import('/runtime/workers/collab-node/dist/server.js');" \ | |||||||||||
| && test -e /runtime/node_modules/@geolibre/collab-core | |||||||||||
|
|
|||||||||||
| FROM node:22-bookworm-slim | |||||||||||
| ENV NODE_ENV=production PORT=8787 COLLAB_DB_PATH=/data/collab.sqlite | |||||||||||
| WORKDIR /app | |||||||||||
| # --chown on the COPY rather than a recursive chown afterwards: chowning /app | |||||||||||
| # rewrites every file, node_modules included, into a second copy in a new layer. | |||||||||||
| COPY --from=build --chown=node:node /app/node_modules ./node_modules | |||||||||||
| COPY --from=build --chown=node:node /app/workers/collab-node/package.json ./workers/collab-node/package.json | |||||||||||
| COPY --from=build --chown=node:node /app/workers/collab-node/dist ./workers/collab-node/dist | |||||||||||
| COPY --from=build --chown=node:node /runtime ./ | |||||||||||
| # /data is created and owned here so a named volume mounted over it inherits the | |||||||||||
| # ownership; Docker only copies image ownership onto a volume when the path | |||||||||||
| # already exists, otherwise the mountpoint lands root-owned. | |||||||||||
|
|
|||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| #!/usr/bin/env node | ||
| // Smoke-test a *running* GeoLibre collaboration relay: health, session | ||
| // creation, and a WebSocket join round-trip. | ||
| // | ||
| // The relay's worst failure mode is an image that builds clean and then exits at | ||
| // startup because the runtime stage is missing a dependency the bundle imports | ||
| // (GeoLibre#1866). No unit test or type-check sees that, and neither does | ||
| // `docker build` -- only starting the container does. So CI builds the image, | ||
| // runs it, and points this script at it. It talks plain HTTP plus the global | ||
| // WebSocket, so it needs nothing installed beyond Node itself and can be aimed | ||
| // at any deployed relay: | ||
| // | ||
| // node workers/collab-node/scripts/smoke.mjs http://127.0.0.1:8787 | ||
|
|
||
| const baseUrl = (process.argv[2] ?? "http://127.0.0.1:8787").replace(/\/+$/, ""); | ||
| // Generous: on a cold CI runner the container has to start Node and open the | ||
| // SQLite database before it listens. | ||
| const STARTUP_TIMEOUT_MS = 60_000; | ||
| const WS_TIMEOUT_MS = 15_000; | ||
|
|
||
| function fail(message, detail) { | ||
| console.error(`FAIL: ${message}`); | ||
| if (detail !== undefined) console.error(detail); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); | ||
|
|
||
| async function waitForHealth() { | ||
| const deadline = Date.now() + STARTUP_TIMEOUT_MS; | ||
| let lastError = "no response"; | ||
| while (Date.now() < deadline) { | ||
| try { | ||
| const response = await fetch(`${baseUrl}/health`); | ||
| const body = await response.json(); | ||
| if (response.ok && body?.ok) return body; | ||
| lastError = `HTTP ${response.status} ${JSON.stringify(body)}`; | ||
| } catch (error) { | ||
| lastError = error instanceof Error ? error.message : String(error); | ||
| } | ||
| await sleep(500); | ||
| } | ||
| fail(`the relay never answered GET /health at ${baseUrl}`, lastError); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| async function createSession() { | ||
| const response = await fetch(`${baseUrl}/sessions`, { | ||
| method: "POST", | ||
| headers: { "content-type": "application/json" }, | ||
| body: "{}", | ||
| }); | ||
| const body = await response.json().catch(() => null); | ||
| if (!response.ok || !body?.sessionId || !body?.hostToken) | ||
| fail( | ||
| "POST /sessions did not return a session", | ||
| `HTTP ${response.status} ${JSON.stringify(body)}`, | ||
| ); | ||
| return body; | ||
| } | ||
|
|
||
| // The join round-trip is the part that actually exercises `ws`: the relay only | ||
| // reaches WebSocketServer.handleUpgrade here, so a missing or broken copy of it | ||
| // shows up as a failed upgrade rather than a passing health check. | ||
| function join(session) { | ||
| const wsUrl = `${baseUrl.replace(/^http/, "ws")}/sessions/${session.sessionId}/ws`; | ||
| return new Promise((resolve) => { | ||
| const socket = new WebSocket(wsUrl); | ||
| const timer = setTimeout(() => { | ||
| socket.close(); | ||
| fail(`no welcome frame within ${WS_TIMEOUT_MS}ms of joining ${wsUrl}`); | ||
| }, WS_TIMEOUT_MS); | ||
|
|
||
| socket.addEventListener("open", () => { | ||
| socket.send( | ||
| JSON.stringify({ | ||
| type: "join", | ||
| clientId: "smoke-test", | ||
| displayName: "Smoke test", | ||
| color: "#2563eb", | ||
| hostToken: session.hostToken, | ||
| }), | ||
| ); | ||
| }); | ||
| socket.addEventListener("message", (event) => { | ||
| clearTimeout(timer); | ||
| let message; | ||
| try { | ||
| message = JSON.parse(String(event.data)); | ||
| } catch { | ||
| fail("the relay sent a frame that is not JSON", String(event.data).slice(0, 200)); | ||
| } | ||
| if (message.type !== "welcome") | ||
| fail( | ||
| `expected a welcome frame, got "${message.type}"`, | ||
| JSON.stringify(message).slice(0, 200), | ||
| ); | ||
| if (message.role !== "host") | ||
| fail(`the host token did not claim the host role (got "${message.role}")`); | ||
| socket.close(); | ||
| resolve(message); | ||
| }); | ||
| socket.addEventListener("error", () => { | ||
| clearTimeout(timer); | ||
| fail(`the WebSocket upgrade to ${wsUrl} failed`); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| const health = await waitForHealth(); | ||
| console.log(`ok GET /health -> ${JSON.stringify(health)}`); | ||
| const session = await createSession(); | ||
| console.log(`ok POST /sessions -> ${session.sessionId} (${session.mode})`); | ||
| const welcome = await join(session); | ||
| console.log( | ||
| `ok WebSocket join -> welcome as ${welcome.role}, ${welcome.participants.length} participant(s)`, | ||
| ); | ||
| console.log(`PASS: the relay at ${baseUrl} is serving`); | ||
Uh oh!
There was an error while loading. Please reload this page.