Skip to content

Commit 8f47cef

Browse files
committed
fix(worker): terminal WebSocket path matches Cloudflare container demo
- Move DO stub.fetch out of Effect so upgrade Response reaches the browser intact. - Split prepareTerminalForWebSocketContext (auth + readiness) from raw stub.fetch. - Strip ticket from container-bound URL; copy WS handshake headers explicitly. - Log DO fetch results and container errors; expose toErrorResponse for non-Hono paths. Made-with: Cursor
1 parent eff33ec commit 8f47cef

4 files changed

Lines changed: 177 additions & 47 deletions

File tree

worker/effect/programs.ts

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ export const requireAuthorizedUsername = (input: {
2222
return yield* auth.requireUsername(input);
2323
});
2424

25-
export const connectTerminal = (input: {
25+
/** Auth, workspace, container readiness — no DO stub.fetch. Cloudflare terminal demo returns stub.fetch outside any framework; Effect breaks WS Responses. */
26+
export const prepareTerminalForWebSocketContext = (input: {
2627
readonly request: Request;
2728
readonly upgrade?: string;
2829
readonly userIdHeader?: string | null;
@@ -75,11 +76,31 @@ export const connectTerminal = (input: {
7576
tabs: selection.tabs,
7677
});
7778

78-
return yield* runtime.proxyTerminalRequest(ready, {
79-
request: input.request,
79+
return {
80+
ready,
8081
username,
8182
sessionId: selection.session.id,
8283
tabId: selection.tab.id,
84+
};
85+
});
86+
87+
/** Tests / legacy: full path including proxy inside Effect (WS Response may not survive). */
88+
export const connectTerminal = (input: {
89+
readonly request: Request;
90+
readonly upgrade?: string;
91+
readonly userIdHeader?: string | null;
92+
readonly ticket?: string | null;
93+
readonly requestedSessionId?: string | null;
94+
readonly requestedTabId?: string | null;
95+
}) =>
96+
Effect.gen(function* () {
97+
const ctx = yield* prepareTerminalForWebSocketContext(input);
98+
const runtime = yield* ContainerRuntime;
99+
return yield* runtime.proxyTerminalRequest(ctx.ready, {
100+
request: input.request,
101+
username: ctx.username,
102+
sessionId: ctx.sessionId,
103+
tabId: ctx.tabId,
83104
});
84105
});
85106

worker/effect/runtime.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ export async function runRequestEffect<A>(
7272
return value;
7373
}
7474

75-
export function toRouteErrorResponse(c: WorkerContext, error: unknown): Response {
75+
export function toErrorResponse(error: unknown): Response {
7676
const normalized = normalizeRouteError(error);
7777
const appError = isAppError(normalized)
7878
? normalized
@@ -96,6 +96,10 @@ export function toRouteErrorResponse(c: WorkerContext, error: unknown): Response
9696
});
9797
}
9898

99+
export function toRouteErrorResponse(_c: WorkerContext, error: unknown): Response {
100+
return toErrorResponse(error);
101+
}
102+
99103
function tryGetExecutionCtx(c: WorkerContext): ExecutionContext | undefined {
100104
try {
101105
return c.executionCtx;

worker/effect/services.ts

Lines changed: 66 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,41 @@ function toPersistenceFailure(message: string) {
177177
}
178178

179179
function toContainerFailure(message: string, retryable: boolean) {
180-
return () => new ContainerUnavailable({ message, retryable });
180+
return (cause: unknown) => {
181+
console.error('[container]', message, { retryable, cause });
182+
return new ContainerUnavailable({ message, retryable });
183+
};
184+
}
185+
186+
/** DO → container: minimal path, no ticket query (JWT length breaks some internal hops). Session/tab come from X-* headers (main.go). */
187+
export function buildContainerWebSocketRequest(
188+
clientRequest: Request,
189+
username: string,
190+
sessionId: string,
191+
tabId: string
192+
): Request {
193+
const internal = new URL('/ws/terminal', 'http://127.0.0.1:8080');
194+
const h = new Headers();
195+
for (const name of [
196+
'upgrade',
197+
'connection',
198+
'sec-websocket-key',
199+
'sec-websocket-version',
200+
'sec-websocket-protocol',
201+
'sec-websocket-extensions',
202+
]) {
203+
const v = clientRequest.headers.get(name);
204+
if (v) {
205+
h.set(name, v);
206+
}
207+
}
208+
h.set('X-User', username);
209+
h.set('X-Session-Id', sessionId);
210+
h.set('X-Tab-Id', tabId);
211+
if (!h.has('sec-websocket-key') || !h.has('sec-websocket-version')) {
212+
console.error('[ws/terminal] missing WS handshake headers on container request');
213+
}
214+
return new Request(internal, { method: 'GET', headers: h });
181215
}
182216

183217
function runtimeAnnotations(context: RuntimeLogContext, containerId: string) {
@@ -871,14 +905,37 @@ const ContainerRuntimeLive = Layer.effect(
871905
ready.containerId,
872906
Effect.tryPromise({
873907
try: async () => {
874-
const headers = new Headers(input.request.headers);
875-
headers.set('X-User', input.username);
876-
headers.set('X-Session-Id', input.sessionId);
877-
headers.set('X-Tab-Id', input.tabId);
878-
// Match @cloudflare/containers: Container.fetch → containerFetch uses getTcpPort + request.url (https→http).
879-
// containers-demos/terminal uses containerFetch(request, defaultPort); WebSockets need that path, not a manual localhost URL.
880-
const proxied = new Request(input.request, { headers });
881-
return ready.container.fetch(switchPort(proxied, 8080));
908+
const t0 = Date.now();
909+
const containerReq = buildContainerWebSocketRequest(
910+
input.request,
911+
input.username,
912+
input.sessionId,
913+
input.tabId
914+
);
915+
const switched = switchPort(containerReq, 8080);
916+
let res: Response;
917+
try {
918+
res = await ready.container.fetch(switched);
919+
} catch (err) {
920+
console.error('[ws/terminal] DO stub.fetch threw', {
921+
containerId: ready.containerId,
922+
ms: Date.now() - t0,
923+
err,
924+
});
925+
throw err;
926+
}
927+
const ws = (res as { webSocket?: WebSocket }).webSocket;
928+
console.log('[ws/terminal] DO stub.fetch result', {
929+
containerId: ready.containerId,
930+
status: res.status,
931+
hasWebSocket: ws != null,
932+
ms: Date.now() - t0,
933+
});
934+
if (res.status >= 400 || (res.status !== 101 && ws == null)) {
935+
const peek = await res.clone().text().catch(() => '');
936+
console.log('[ws/terminal] non-upgrade response body', peek.slice(0, 400));
937+
}
938+
return res;
882939
},
883940
catch: toContainerFailure('Container error, please retry in a moment.', true),
884941
})

worker/index.ts

Lines changed: 82 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
import { Hono } from 'hono';
2-
import type { Context } from 'hono';
3-
import { Container, getContainer } from '@cloudflare/containers';
2+
import type { Context, ExecutionContext } from 'hono';
3+
import { Container, getContainer, switchPort } from '@cloudflare/containers';
44
import {
55
backupWorkspace,
66
checkpointSession,
7-
connectTerminal,
87
createLegacyTab,
98
createSession,
109
createTab,
@@ -21,8 +20,17 @@ import {
2120
requireAuthorizedUsername,
2221
updateTab,
2322
updateSession,
23+
prepareTerminalForWebSocketContext,
2424
} from './effect/programs';
25-
import { runJsonRoute, runRequestEffect, runRouteEffect, toRouteErrorResponse } from './effect/runtime';
25+
import { buildContainerWebSocketRequest } from './effect/services';
26+
import {
27+
runJsonRoute,
28+
runRequestEffect,
29+
runRouteEffect,
30+
toErrorResponse,
31+
toRouteErrorResponse,
32+
} from './effect/runtime';
33+
import { UnexpectedFailure } from './effect/errors';
2634
import { getUserSessionContainerId, readWorkerIdentity } from './auth';
2735
import { isContainerActiveStatus } from './tabs';
2836
import type { Env } from './types';
@@ -154,35 +162,6 @@ function createApp() {
154162
// Health check
155163
app.get('/health', (c) => c.json({ status: 'ok' }));
156164

157-
// WebSocket terminal with Effect orchestration
158-
app.get('/ws/terminal', (c) => {
159-
const upgrade = c.req.header('Upgrade');
160-
if (upgrade?.toLowerCase() !== 'websocket') {
161-
return c.text('expected websocket', 426);
162-
}
163-
164-
// Workers Logs (not Container stdout): confirms the request reached this worker before DO/container.
165-
console.log('[ws/terminal] incoming', {
166-
hasTicket: Boolean(c.req.query('ticket')),
167-
querySessionId: c.req.query('sessionId') ?? null,
168-
queryTabId: c.req.query('tabId') ?? null,
169-
hasUserHeader: Boolean(c.req.header('X-User-Id')),
170-
});
171-
172-
return runRouteEffect(
173-
c,
174-
connectTerminal({
175-
request: c.req.raw,
176-
upgrade,
177-
userIdHeader: c.req.header('X-User-Id'),
178-
ticket: c.req.query('ticket'),
179-
requestedSessionId: c.req.query('sessionId'),
180-
requestedTabId: c.req.query('tabId'),
181-
}),
182-
(response) => response
183-
);
184-
});
185-
186165
// Session APIs
187166
app.get('/api/sessions', (c) => runJsonRoute(c, listSessions(c.req.header('X-User-Id'))));
188167
app.post('/api/sessions', (c) =>
@@ -514,5 +493,74 @@ function createApp() {
514493

515494
const app = createApp();
516495

496+
async function handleTerminalWebSocket(request: Request, env: Env): Promise<Response> {
497+
const url = new URL(request.url);
498+
console.log('[ws/terminal] incoming (raw)', {
499+
hasTicket: Boolean(url.searchParams.get('ticket')),
500+
querySessionId: url.searchParams.get('sessionId'),
501+
queryTabId: url.searchParams.get('tabId'),
502+
hasUserHeader: Boolean(request.headers.get('X-User-Id')),
503+
});
504+
505+
try {
506+
// containers-demos/terminal: `return await getContainer(env.TERMINAL).fetch(request)` — no Effect around stub.fetch.
507+
const prep = await runRequestEffect(
508+
env,
509+
prepareTerminalForWebSocketContext({
510+
request,
511+
upgrade: request.headers.get('Upgrade') ?? undefined,
512+
userIdHeader: request.headers.get('X-User-Id'),
513+
ticket: url.searchParams.get('ticket'),
514+
requestedSessionId: url.searchParams.get('sessionId'),
515+
requestedTabId: url.searchParams.get('tabId'),
516+
})
517+
);
518+
519+
const t0 = Date.now();
520+
const inner = buildContainerWebSocketRequest(request, prep.username, prep.sessionId, prep.tabId);
521+
let res: Response;
522+
try {
523+
res = await prep.ready.container.fetch(switchPort(inner, 8080));
524+
} catch (err) {
525+
console.error('[ws/terminal] DO stub.fetch threw', { ms: Date.now() - t0, err });
526+
return toErrorResponse(
527+
new UnexpectedFailure({
528+
message: 'Container error, please retry in a moment.',
529+
cause: err,
530+
})
531+
);
532+
}
533+
534+
const ws = (res as { webSocket?: WebSocket }).webSocket;
535+
console.log('[ws/terminal] direct stub.fetch', {
536+
status: res.status,
537+
hasWebSocket: ws != null,
538+
ms: Date.now() - t0,
539+
containerId: prep.ready.containerId,
540+
});
541+
if (res.status >= 400 || (res.status !== 101 && ws == null)) {
542+
const peek = await res.clone().text().catch(() => '');
543+
console.log('[ws/terminal] non-upgrade response body', peek.slice(0, 400));
544+
}
545+
return res;
546+
} catch (error) {
547+
return toErrorResponse(error);
548+
}
549+
}
550+
517551
export { createApp };
518-
export default app;
552+
export default {
553+
async fetch(request: Request, env: Env, ctx: ExecutionContext): Promise<Response> {
554+
const url = new URL(request.url);
555+
if (url.pathname === '/ws/terminal' && request.method === 'GET') {
556+
if (request.headers.get('Upgrade')?.toLowerCase() !== 'websocket') {
557+
return new Response('expected websocket', {
558+
status: 426,
559+
headers: { 'Content-Type': 'text/plain; charset=UTF-8' },
560+
});
561+
}
562+
return handleTerminalWebSocket(request, env);
563+
}
564+
return app.fetch(request, env, ctx);
565+
},
566+
};

0 commit comments

Comments
 (0)