Skip to content

Commit 854956a

Browse files
committed
refactor(worker): simplify WebSocket URL resolution for terminal connections
- Replaced manual URL construction with a dedicated function to resolve WebSocket client URLs, improving code clarity and maintainability. - Updated the test command in package.json to include additional test execution for shared components. Made-with: Cursor
1 parent 71f22cd commit 854956a

4 files changed

Lines changed: 60 additions & 14 deletions

File tree

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
"db:studio": "drizzle-kit studio",
1919
"db:local": "wrangler d1 migrations apply cloudshell-db --local",
2020
"db:remote": "wrangler d1 migrations apply cloudshell-db --remote",
21-
"test": "vitest run",
21+
"test": "vitest run && bun test shared/",
2222
"test:watch": "vitest"
2323
},
2424
"devDependencies": {
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { describe, expect, it } from 'bun:test';
2+
import { resolveTerminalWebSocketClientUrl } from './resolve-terminal-ws-url';
3+
4+
describe('resolveTerminalWebSocketClientUrl', () => {
5+
it('prod: same-origin WSS on app host, proxy mode (not API subdomain)', () => {
6+
const apiOrigin = 'https://cloudshell-api.example.com';
7+
const { url, mode } = resolveTerminalWebSocketClientUrl({
8+
dev: false,
9+
appOrigin: 'https://cloudshell.example.com',
10+
workerDevOrigin: 'unused',
11+
ticket: 'a.b',
12+
});
13+
14+
expect(mode).toBe('proxy');
15+
expect(url.startsWith('wss://cloudshell.example.com/ws/terminal?')).toBe(true);
16+
expect(url).toContain('ticket=a.b');
17+
expect(url).not.toContain(apiOrigin);
18+
expect(url).not.toContain('cloudshell-api');
19+
});
20+
21+
it('dev: WS URL targets worker dev origin, direct mode', () => {
22+
const { url, mode } = resolveTerminalWebSocketClientUrl({
23+
dev: true,
24+
appOrigin: 'https://unused',
25+
workerDevOrigin: 'http://localhost:1338',
26+
ticket: 't',
27+
});
28+
29+
expect(mode).toBe('direct');
30+
expect(url).toBe('ws://localhost:1338/ws/terminal?ticket=t');
31+
});
32+
});

shared/resolve-terminal-ws-url.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/** Pure URL builder for the browser terminal WebSocket (no Svelte / Workers imports). */
2+
3+
export function resolveTerminalWebSocketClientUrl(input: {
4+
readonly dev: boolean;
5+
readonly appOrigin: string;
6+
readonly workerDevOrigin: string;
7+
readonly ticket: string;
8+
}): { readonly url: string; readonly mode: 'proxy' | 'direct' } {
9+
if (input.dev) {
10+
const url = new URL('/ws/terminal', input.workerDevOrigin);
11+
url.protocol = url.protocol === 'https:' ? 'wss:' : 'ws:';
12+
url.searchParams.set('ticket', input.ticket);
13+
return { url: url.toString(), mode: 'direct' };
14+
}
15+
16+
const url = new URL('/ws/terminal', input.appOrigin);
17+
url.protocol = url.protocol === 'https:' ? 'wss:' : 'ws:';
18+
url.searchParams.set('ticket', input.ticket);
19+
return { url: url.toString(), mode: 'proxy' };
20+
}

src/lib/server/worker.ts

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { dev } from '$app/environment';
22
import { error, type RequestEvent } from '@sveltejs/kit';
3+
import { resolveTerminalWebSocketClientUrl } from '../../../shared/resolve-terminal-ws-url';
34
import { createTerminalTicket } from '../../../shared/terminal-ticket';
45

56
const DEFAULT_DEV_WORKER_ORIGIN = 'http://localhost:1338';
@@ -119,17 +120,10 @@ export async function resolveTerminalConnection(
119120
secret
120121
);
121122

122-
if (dev) {
123-
const url = new URL('/ws/terminal', getWorkerOrigin(event));
124-
url.protocol = url.protocol === 'https:' ? 'wss:' : 'ws:';
125-
url.searchParams.set('ticket', ticket);
126-
return { url: url.toString(), mode: 'direct' };
127-
}
128-
129-
// Same-origin WSS: browser → Pages `/ws/terminal` → `WORKER.fetch` → API worker (`proxyTerminalWebSocket`).
130-
// Avoids cross-host upgrades (app vs api) which often surface as 1006 with little detail.
131-
const url = new URL('/ws/terminal', event.url.origin);
132-
url.protocol = url.protocol === 'https:' ? 'wss:' : 'ws:';
133-
url.searchParams.set('ticket', ticket);
134-
return { url: url.toString(), mode: 'proxy' };
123+
return resolveTerminalWebSocketClientUrl({
124+
dev,
125+
appOrigin: event.url.origin,
126+
workerDevOrigin: getWorkerOrigin(event),
127+
ticket,
128+
});
135129
}

0 commit comments

Comments
 (0)