diff --git a/examples/socket/README.md b/examples/socket/README.md index 9b95c1b3b..9c8004c91 100644 --- a/examples/socket/README.md +++ b/examples/socket/README.md @@ -15,8 +15,9 @@ Socket Mode bots should not be submitted to Marketplace for publishing. - **Enabling Socket Mode** — `new App({ socketMode: true })` for WebSocket inbound delivery with no HTTP endpoint. - **Multi-geo by default** — a single bot opens one connection per geo - (`amer`, `emea`, `apac`) so it has inbound coverage across regions. Override - with `geos: [...]` or point at a custom ring with `negotiateBaseUrl`. + (`amer`, `emea`, `apac`) so it has inbound coverage across regions. Supply + unique, non-empty hostname prefixes with `geos: [...]`; an empty list keeps + the defaults. - **Lifecycle events** — subscribing to `app.socketMode.events` for `ready`, `disconnected`, and `reconnected`. Each event carries the `geo` it relates to, since connections are per geo. Reconnects are automatic; the events are purely diff --git a/packages/apps/src/socket-mode/app.e2e.protocol-observability.spec.ts b/packages/apps/src/socket-mode/app.e2e.protocol-observability.spec.ts index b78abdc2b..98ac1b920 100644 --- a/packages/apps/src/socket-mode/app.e2e.protocol-observability.spec.ts +++ b/packages/apps/src/socket-mode/app.e2e.protocol-observability.spec.ts @@ -322,9 +322,13 @@ describe('Socket Mode E2E: protocol and observability', () => { await app.stop(); }); - it('exposes geo-tagged lifecycle events', async () => { + it.each([ + ['amer', 'https://amer.botapi.skype.com/v3/websockets/connect'], + ['emea', 'https://emea.botapi.skype.com/v3/websockets/connect'], + ['apac', 'https://apac.botapi.skype.com/v3/websockets/connect'], + ])('exposes %s negotiate and geo-tagged lifecycle observability', async (geo, url) => { const app = createSocketTestApp({ - socketMode: { geos: [''], reconnectDelaysMs: [0] }, + socketMode: { geos: [geo], reconnectDelaysMs: [0] }, }); const ready = jest.fn(); const disconnected = jest.fn(); @@ -334,16 +338,18 @@ describe('Socket Mode E2E: protocol and observability', () => { app.socketMode!.events.on('reconnected', reconnected); await app.start(); + expect(connectionState.contexts[0].negotiateUrl).toBe(url); + const error = new Error('network drop'); connectionState.connections[0].drop(error); await new Promise((resolve) => setTimeout(resolve, 0)); expect(ready).toHaveBeenCalledWith({ - geo: '', + geo, frame: expect.objectContaining({ botKey: 'bot1' }), }); - expect(disconnected).toHaveBeenCalledWith({ geo: '', error, terminal: false }); - expect(reconnected).toHaveBeenCalledWith({ geo: '' }); + expect(disconnected).toHaveBeenCalledWith({ geo, error, terminal: false }); + expect(reconnected).toHaveBeenCalledWith({ geo }); await app.stop(); }); }); diff --git a/packages/apps/src/socket-mode/app.e2e.test-harness.ts b/packages/apps/src/socket-mode/app.e2e.test-harness.ts index afca61a45..f8eb4b946 100644 --- a/packages/apps/src/socket-mode/app.e2e.test-harness.ts +++ b/packages/apps/src/socket-mode/app.e2e.test-harness.ts @@ -167,14 +167,13 @@ export function createSocketTestApp( socketMode?: boolean | (SocketModeOptions & SocketModeTestSettings); } = {} ): App { - const { socketMode = { geos: [''] }, ...appOptions } = options; - const { geos, negotiateBaseUrl, ...settings } = - typeof socketMode === 'object' ? socketMode : {}; + const { socketMode = { geos: ['amer'] }, ...appOptions } = options; + const { geos, ...settings } = typeof socketMode === 'object' ? socketMode : {}; const app = createTestApp({ clientId: 'bot1', logger: quietLogger(), ...appOptions, - socketMode: typeof socketMode === 'object' ? { geos, negotiateBaseUrl } : socketMode, + socketMode: typeof socketMode === 'object' ? { geos } : socketMode, }); if (app.socketMode) configureSocketModeForTest(app.socketMode, settings); return app; diff --git a/packages/apps/src/socket-mode/app.e2e.transport-recovery.spec.ts b/packages/apps/src/socket-mode/app.e2e.transport-recovery.spec.ts index c89348fdf..0d3a2267c 100644 --- a/packages/apps/src/socket-mode/app.e2e.transport-recovery.spec.ts +++ b/packages/apps/src/socket-mode/app.e2e.transport-recovery.spec.ts @@ -92,7 +92,7 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' it('renegotiates a fresh connection after an unexpected drop', async () => { const app = createSocketTestApp({ - socketMode: { geos: [''], reconnectDelaysMs: [0] }, + socketMode: { geos: ['amer'], reconnectDelaysMs: [0] }, }); const disconnected = jest.fn(); const reconnected = jest.fn(); @@ -105,8 +105,8 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' await ticks(); expect(connectionState.connections).toHaveLength(2); - expect(disconnected).toHaveBeenCalledWith({ geo: '', error: dropped, terminal: false }); - expect(reconnected).toHaveBeenCalledWith({ geo: '' }); + expect(disconnected).toHaveBeenCalledWith({ geo: 'amer', error: dropped, terminal: false }); + expect(reconnected).toHaveBeenCalledWith({ geo: 'amer' }); expect(app.socketMode!.status).toBe('ready'); await app.stop(); }); @@ -152,7 +152,7 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' connectionState.autoReadyQueue = [true, false]; const handler = jest.fn(); const app = createSocketTestApp({ - socketMode: { geos: [''], reconnectDelaysMs: [0] }, + socketMode: { geos: ['amer'], reconnectDelaysMs: [0] }, }); app.on('message', handler); await app.start(); @@ -197,7 +197,7 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' let release!: () => void; const gate = new Promise((resolve) => { release = resolve; }); const app = createSocketTestApp({ - socketMode: { geos: [''], reconnectDelaysMs: [0] }, + socketMode: { geos: ['amer'], reconnectDelaysMs: [0] }, }); app.on('message', () => gate); await app.start(); @@ -232,7 +232,7 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' const firstGate = new Promise((resolve) => { finishFirst = resolve; }); const secondGate = new Promise((resolve) => { finishSecond = resolve; }); const app = createSocketTestApp({ - socketMode: { geos: [''], reconnectDelaysMs: [0] }, + socketMode: { geos: ['amer'], reconnectDelaysMs: [0] }, }); app.on('message', ({ activity }) => activity.id === 'first' ? firstGate : secondGate); const disconnected = jest.fn(); @@ -297,7 +297,7 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' connectionState.expiresInSeconds = 120; const releases = new Map void>(); const app = createSocketTestApp({ - socketMode: { geos: [''], reconnectDelaysMs: [0] }, + socketMode: { geos: ['amer'], reconnectDelaysMs: [0] }, }); app.on('message', ({ activity }) => new Promise((resolve) => { releases.set(activity.id, resolve); }) @@ -352,7 +352,7 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' let release!: () => void; const gate = new Promise((resolve) => { release = resolve; }); const app = createSocketTestApp({ - socketMode: { geos: [''], reconnectDelaysMs: [0] }, + socketMode: { geos: ['amer'], reconnectDelaysMs: [0] }, }); app.on('message', () => gate); const disconnected = jest.fn(); @@ -393,7 +393,7 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' connectionState.expiresInSeconds = 120; const handler = jest.fn(); const app = createSocketTestApp({ - socketMode: { geos: [''], reconnectDelaysMs: [100] }, + socketMode: { geos: ['amer'], reconnectDelaysMs: [100] }, }); app.on('message', handler); await app.start(); @@ -439,7 +439,7 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' connectionState.expiresInSeconds = 120; connectionState.autoReadyQueue = [true, false]; const app = createSocketTestApp({ - socketMode: { geos: [''], reconnectDelaysMs: [0] }, + socketMode: { geos: ['amer'], reconnectDelaysMs: [0] }, }); const disconnected = jest.fn(); const reconnected = jest.fn(); @@ -455,13 +455,13 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' oldConnection.drop(error); expect(app.socketMode!.status).toBe('disconnected'); - expect(disconnected).toHaveBeenCalledWith({ geo: '', error, terminal: false }); + expect(disconnected).toHaveBeenCalledWith({ geo: 'amer', error, terminal: false }); replacement.fireReady(); await jest.advanceTimersByTimeAsync(0); expect(app.socketMode!.status).toBe('ready'); - expect(reconnected).toHaveBeenCalledWith({ geo: '' }); + expect(reconnected).toHaveBeenCalledWith({ geo: 'amer' }); await app.stop(); } finally { jest.useRealTimers(); @@ -487,14 +487,13 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' await defaultApp.start(); expect(connectionState.contexts[0]).toMatchObject({ - negotiateUrl: 'https://botapi.skype.com/v3/websockets/connect', + negotiateUrl: 'https://amer.botapi.skype.com/v3/websockets/connect', readinessTimeoutMs: 30_000, }); await defaultApp.stop(); const unsupportedOptions = { - geos: [''], - negotiateBaseUrl: 'https://apx.example/ring/', + geos: ['amer'], readinessTimeoutMs: 1234, keepAliveIntervalMs: 7000, serverTimeoutMs: 9000, @@ -506,7 +505,7 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' await customApp.start(); expect(connectionState.contexts[1]).toMatchObject({ - negotiateUrl: 'https://apx.example/ring/v3/websockets/connect', + negotiateUrl: 'https://amer.botapi.skype.com/v3/websockets/connect', getConnectionIdToReplace: undefined, readinessTimeoutMs: 30_000, }); @@ -524,7 +523,7 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' jest.useFakeTimers(); const random = jest.spyOn(Math, 'random').mockReturnValue(0.5); const unsupportedOptions = { - geos: [''], + geos: ['amer'], startupTimeoutMs: 0, reconnectDelaysMs: [0], }; @@ -554,7 +553,7 @@ describe('Socket Mode E2E: exclusive transport, failure, ordering, and recovery' it('does not let a runtime override extend the 30-second startup budget', async () => { jest.useFakeTimers(); - const unsupportedOptions = { geos: [''], startupTimeoutMs: 60_000 }; + const unsupportedOptions = { geos: ['amer'], startupTimeoutMs: 60_000 }; const app = createTestApp({ clientId: 'bot1', socketMode: unsupportedOptions }); try { connectionState.startErrorQueue.push( diff --git a/packages/apps/src/socket-mode/negotiate.spec.ts b/packages/apps/src/socket-mode/negotiate.spec.ts index 5b15fe10c..92476084a 100644 --- a/packages/apps/src/socket-mode/negotiate.spec.ts +++ b/packages/apps/src/socket-mode/negotiate.spec.ts @@ -97,6 +97,31 @@ describe('socket-mode negotiate', () => { expect(diagnostics).toContain('[REDACTED]'); }); + it('sanitizes the negotiate URL in diagnostics without changing the request URL', async () => { + const originalUrl = + 'https://diagnostic-user:diagnostic-pass@apx.example/v3/websockets/connect?sig=query-secret#fragment-secret'; + const log = { + debug: jest.fn(), + }; + post.mockResolvedValue( + httpResponse({ url: 'wss://sr/hub', accessToken: 'sr-token', expiresIn: 3600 }) + ); + + await negotiate({ + ...deps(), + negotiateUrl: originalUrl, + log: log as any, + }); + + const diagnostics = log.debug.mock.calls.flat().join(' '); + expect(diagnostics).not.toContain('diagnostic-user'); + expect(diagnostics).not.toContain('diagnostic-pass'); + expect(diagnostics).not.toContain('query-secret'); + expect(diagnostics).not.toContain('fragment-secret'); + expect(diagnostics).toContain('https://apx.example/v3/websockets/connect'); + expect(post.mock.calls[0][0]).toBe(originalUrl); + }); + it('parses Retry-After (delta-seconds) into NegotiateError.retryAfterMs on a 429', async () => { post.mockResolvedValue( httpResponse('slow down', 429, { diff --git a/packages/apps/src/socket-mode/negotiate.ts b/packages/apps/src/socket-mode/negotiate.ts index c41b0e08c..9e87559a6 100644 --- a/packages/apps/src/socket-mode/negotiate.ts +++ b/packages/apps/src/socket-mode/negotiate.ts @@ -1,6 +1,6 @@ import { Client as HttpClient, ILogger } from '@microsoft/teams.common'; -import { redactSocketModeSecrets } from './redact'; +import { sanitizeSocketModeUrl, redactSocketModeSecrets } from './redact'; import { NegotiateResult } from './types'; /** @@ -20,9 +20,9 @@ export type NegotiateDeps = { */ readonly getConnectionIdToReplace?: () => string | undefined; /** - * Abort the negotiate request after this many milliseconds so a bad - * `negotiateBaseUrl` (or a hung endpoint) can't leave the caller pending - * forever. Defaults to {@link DEFAULT_NEGOTIATE_TIMEOUT_MS}. + * Abort the negotiate request after this many milliseconds so a hung endpoint + * can't leave the caller pending forever. Defaults to + * {@link DEFAULT_NEGOTIATE_TIMEOUT_MS}. */ readonly timeoutMs?: number; readonly log?: ILogger; @@ -143,7 +143,7 @@ export async function negotiate(deps: NegotiateDeps): Promise { assertSecureNegotiateUrl(deps.negotiateUrl); - deps.log?.debug(`socket-mode: negotiate POST ${deps.negotiateUrl}`); + deps.log?.debug(`socket-mode: negotiate POST ${sanitizeSocketModeUrl(deps.negotiateUrl)}`); const replacesConnectionId = deps.getConnectionIdToReplace?.(); const res = await deps.client.post>( diff --git a/packages/apps/src/socket-mode/redact.ts b/packages/apps/src/socket-mode/redact.ts index ea4207800..1d7aa2d80 100644 --- a/packages/apps/src/socket-mode/redact.ts +++ b/packages/apps/src/socket-mode/redact.ts @@ -1,5 +1,15 @@ const REDACTED = '[REDACTED]'; +/** Removes credentials, query parameters, and fragments from a URL used in diagnostics. */ +export function sanitizeSocketModeUrl(value: string): string { + const url = new URL(value); + url.username = ''; + url.password = ''; + url.search = ''; + url.hash = ''; + return url.toString(); +} + /** * Removes Socket Mode credentials from diagnostic text before it reaches logs * or caller-visible errors. diff --git a/packages/apps/src/socket-mode/socket-connection.spec.ts b/packages/apps/src/socket-mode/socket-connection.spec.ts index 168b65117..4f5231d45 100644 --- a/packages/apps/src/socket-mode/socket-connection.spec.ts +++ b/packages/apps/src/socket-mode/socket-connection.spec.ts @@ -101,7 +101,7 @@ const client = { post } as unknown as HttpClient; function makeContext(overrides: Partial = {}): SocketConnectionContext { return { - negotiateUrl: 'https://apx.example/v3/websockets/connect', + negotiateUrl: 'https://amer.botapi.skype.com/v3/websockets/connect', client, getBotToken: async () => 'bot-jwt', readinessTimeoutMs: 50, @@ -119,10 +119,7 @@ function makeHandlers(overrides: Partial = {}): Socket } function makeAdapter(getBotToken = async () => 'bot-jwt', reconnectDelayMs = 0): SocketModeAdapter { - const adapter = new SocketModeAdapter({ - negotiateBaseUrl: 'https://apx.example', - geos: ['amer'], - }, { + const adapter = new SocketModeAdapter({ geos: ['amer'] }, { client, tokenProvider: { getAppToken: async () => { @@ -186,7 +183,9 @@ describe('SignalRSocketConnection', () => { it('keeps server IDs independent across overlapping sockets', async () => { const first = new SignalRSocketConnection( - makeContext({ negotiateUrl: 'https://apx.example/amer/v3/websockets/connect' }), + makeContext({ + negotiateUrl: 'https://amer.botapi.skype.com/v3/websockets/connect', + }), makeHandlers() ); await first.start(); @@ -198,7 +197,9 @@ describe('SignalRSocketConnection', () => { headers: {}, }); const next = new SignalRSocketConnection( - makeContext({ negotiateUrl: 'https://apx.example/emea/v3/websockets/connect' }), + makeContext({ + negotiateUrl: 'https://emea.botapi.skype.com/v3/websockets/connect', + }), makeHandlers() ); await next.start(); @@ -224,7 +225,9 @@ describe('SignalRSocketConnection', () => { state.readyFrame = { connectionId: '' }; await jest.advanceTimersByTimeAsync(240_000); expect(post).toHaveBeenCalledTimes(2); - expect(post.mock.calls[1][0]).toBe('https://apx.example/amer/v3/websockets/connect'); + expect(post.mock.calls[1][0]).toBe( + 'https://amer.botapi.skype.com/v3/websockets/connect' + ); expect(post.mock.calls[1][2].headers[header]).toBe(' c1/+== '); expect(state.stopped).toBe(0); diff --git a/packages/apps/src/socket-mode/socket-mode-adapter.resilience.spec.ts b/packages/apps/src/socket-mode/socket-mode-adapter.resilience.spec.ts index e723b32bb..9859d6506 100644 --- a/packages/apps/src/socket-mode/socket-mode-adapter.resilience.spec.ts +++ b/packages/apps/src/socket-mode/socket-mode-adapter.resilience.spec.ts @@ -126,8 +126,8 @@ function createSocketModeAdapter( deps: Omit ): SocketModeAdapter { let handler: ActivityHandler = async () => ({ status: 200 }); - const { geos, negotiateBaseUrl, ...settings } = options; - const server = new SocketModeAdapter({ geos, negotiateBaseUrl }, { + const { geos, ...settings } = options; + const server = new SocketModeAdapter({ geos }, { credentials: { clientId: 'bot1' } as any, client: new HttpClient(), ...deps, @@ -160,10 +160,10 @@ async function makeServer( options: SocketModeOptions & SocketModeTestSettings = {}, logger: ConsoleLogger = new ConsoleLogger('test', { level: 'error' }) ): Promise { - // Default to a single geo ([''] = no geo segment) so the per-connection + // Default to a single geo so the per-connection // supervisor tests operate on exactly one connection; multi-geo behavior has // its own describe block below. - const server = createSocketModeAdapter({ geos: [''], ...options }, { + const server = createSocketModeAdapter({ geos: ['amer'], ...options }, { tokenProvider: { getAppToken: async () => 'app-token' } as any, messagingEndpoint: MESSAGING_ENDPOINT, logger, @@ -238,7 +238,7 @@ describe('SocketModeAdapter resilience', () => { expect(connState.connections).toHaveLength(index + 2); expect(server.status).toBe('ready'); expect(reconnected).toHaveBeenCalledTimes(index + 1); - expect(disconnected).toHaveBeenLastCalledWith({ geo: '', error, terminal: false }); + expect(disconnected).toHaveBeenLastCalledWith({ geo: 'amer', error, terminal: false }); } await jest.advanceTimersByTimeAsync(539_999); @@ -367,7 +367,7 @@ describe('SocketModeAdapter resilience', () => { const recording = new ConsoleLogger('test', { level: 'error' }); (recording as any).warn = (...args: any[]) => warnings.push(args); - const server = createSocketModeAdapter({ geos: [''], reconnectDelaysMs: [0] }, { + const server = createSocketModeAdapter({ geos: ['amer'], reconnectDelaysMs: [0] }, { tokenProvider: { getAppToken: async () => 'app-token' } as any, messagingEndpoint: MESSAGING_ENDPOINT, logger: recording, @@ -517,13 +517,13 @@ describe('SocketModeAdapter resilience', () => { it('closes only the affected geo when a planned refresh is rejected with 403', async () => { jest.useFakeTimers(); try { - connState.expiresInByUrl = { '/amer/': 120 }; + connState.expiresInByUrl = { '//amer.': 120 }; const server = await makeServer({ geos: ['amer', 'emea'], reconnectDelaysMs: [0] }); const handler = jest.fn(async () => ({ status: 200 })); onMessaging(server, handler); await server.start(); const byGeo = (geo: string) => - connState.connections.find((c: any) => c.context.negotiateUrl.includes(`/${geo}/`))!; + connState.connections.find((c: any) => c.context.negotiateUrl.includes(`//${geo}.`))!; const affected = byGeo('amer'); const healthy = byGeo('emea'); @@ -660,6 +660,44 @@ describe('SocketModeAdapter resilience', () => { }); describe('multi-geo', () => { + it('uses custom geo values exactly as supplied for transport URLs', async () => { + const server = createSocketModeAdapter({ geos: ['eur', 'AMER', 'amer.amer'] }, { + tokenProvider: { getAppToken: async () => 'app-token' } as any, + messagingEndpoint: MESSAGING_ENDPOINT, + logger: new ConsoleLogger('test', { level: 'error' }), + }); + onMessaging(server, jest.fn(async () => ({ status: 200 }))); + + await server.start(); + + expect(server.geoList).toEqual(['eur', 'AMER', 'amer.amer']); + expect(connState.connections.map((c: any) => c.context.negotiateUrl)).toEqual([ + 'https://eur.botapi.skype.com/v3/websockets/connect', + 'https://AMER.botapi.skype.com/v3/websockets/connect', + 'https://amer.amer.botapi.skype.com/v3/websockets/connect', + ]); + await server.stop(); + }); + + it('fails startup through the transport when an unsupported backend geo is included', async () => { + const error = new Error('unsupported geo'); + const server = createSocketModeAdapter({ geos: ['unsupported', 'amer'], startupTimeoutMs: 0 }, { + tokenProvider: { getAppToken: async () => 'app-token' } as any, + messagingEndpoint: MESSAGING_ENDPOINT, + logger: new ConsoleLogger('test', { level: 'error' }), + }); + connState.startErrorQueue.push(error); + + await expect(server.start()).rejects.toBe(error); + + expect(server.geoList).toEqual(['unsupported', 'amer']); + expect(connState.connections.map((c: any) => c.context.negotiateUrl)).toEqual([ + 'https://unsupported.botapi.skype.com/v3/websockets/connect', + 'https://amer.botapi.skype.com/v3/websockets/connect', + ]); + expect(server.status).toBe('stopped'); + }); + it('opens one connection per default geo (amer/emea/apac) with geo-scoped URLs', async () => { const server = createSocketModeAdapter({ reconnectDelaysMs: [0] }, { tokenProvider: { getAppToken: async () => 'app-token' } as any, @@ -674,15 +712,57 @@ describe('SocketModeAdapter resilience', () => { expect(connState.connections).toHaveLength(3); const urls = connState.connections.map((c: any) => c.context.negotiateUrl).sort(); expect(urls).toEqual([ - 'https://botapi.skype.com/amer/v3/websockets/connect', - 'https://botapi.skype.com/apac/v3/websockets/connect', - 'https://botapi.skype.com/emea/v3/websockets/connect', + 'https://amer.botapi.skype.com/v3/websockets/connect', + 'https://apac.botapi.skype.com/v3/websockets/connect', + 'https://emea.botapi.skype.com/v3/websockets/connect', ]); expect(server.status).toBe('ready'); await server.stop(); }); + it('uses the default geos for an explicit empty geo list', async () => { + const server = createSocketModeAdapter({ geos: [] }, { + tokenProvider: { getAppToken: async () => 'app-token' } as any, + messagingEndpoint: MESSAGING_ENDPOINT, + logger: new ConsoleLogger('test', { level: 'error' }), + }); + onMessaging(server, jest.fn(async () => ({ status: 200 }))); + + await server.start(); + + expect(server.geoList).toEqual(['amer', 'emea', 'apac']); + expect(connState.connections).toHaveLength(3); + expect(server.status).toBe('ready'); + await server.stop(); + }); + + it('rejects an empty geo before entering connecting state', async () => { + const server = createSocketModeAdapter({ geos: ['amer', ''] }, { + tokenProvider: { getAppToken: async () => 'app-token' } as any, + messagingEndpoint: MESSAGING_ENDPOINT, + logger: new ConsoleLogger('test', { level: 'error' }), + }); + + await expect(server.start()).rejects.toThrow('socketMode.geos[1] must not be empty.'); + expect(server.status).toBe('idle'); + expect(connState.connections).toHaveLength(0); + }); + + it('rejects case-insensitive duplicate geos before entering connecting state', async () => { + const server = createSocketModeAdapter({ geos: ['amer', 'AMER'] }, { + tokenProvider: { getAppToken: async () => 'app-token' } as any, + messagingEndpoint: MESSAGING_ENDPOINT, + logger: new ConsoleLogger('test', { level: 'error' }), + }); + + await expect(server.start()).rejects.toThrow( + 'socketMode.geos must not contain duplicates.' + ); + expect(server.status).toBe('idle'); + expect(connState.connections).toHaveLength(0); + }); + it('reconnects one geo independently without disturbing the others', async () => { const server = createSocketModeAdapter({ reconnectDelaysMs: [0] }, { tokenProvider: { getAppToken: async () => 'app-token' } as any, @@ -755,7 +835,7 @@ describe('SocketModeAdapter resilience', () => { async function makeServerWithOnError( onError: (error: Error) => void | Promise ): Promise { - const server = createSocketModeAdapter({ geos: [''] }, { + const server = createSocketModeAdapter({ geos: ['amer'] }, { tokenProvider: { getAppToken: async () => 'app-token' } as any, messagingEndpoint: MESSAGING_ENDPOINT, onError, diff --git a/packages/apps/src/socket-mode/socket-mode-adapter.spec.ts b/packages/apps/src/socket-mode/socket-mode-adapter.spec.ts index 0e0fa90cc..b5aa38bbe 100644 --- a/packages/apps/src/socket-mode/socket-mode-adapter.spec.ts +++ b/packages/apps/src/socket-mode/socket-mode-adapter.spec.ts @@ -90,7 +90,7 @@ describe('SocketModeAdapter (through App)', () => { const app = createTestApp({ logger: new ConsoleLogger('test', { level: 'error' }), clientId: 'bot1', - socketMode: { geos: [''] }, + socketMode: { geos: ['amer'] }, }); configureSocketModeForTest(app.socketMode!, { startupTimeoutMs: 0 }); @@ -98,14 +98,12 @@ describe('SocketModeAdapter (through App)', () => { expect(app.socketMode?.status).toBe('stopped'); }); - it('rejects sovereign clouds even when a custom negotiate endpoint is supplied', () => { + it('rejects sovereign clouds', () => { expect(() => createTestApp({ cloud: US_GOV, clientId: 'bot1', - socketMode: { - negotiateBaseUrl: 'https://socket.example', - }, + socketMode: true, }) ).toThrow(/supports only regular production clouds/i); }); diff --git a/packages/apps/src/socket-mode/socket-mode-adapter.ts b/packages/apps/src/socket-mode/socket-mode-adapter.ts index 6b5d52ccb..72b206c7f 100644 --- a/packages/apps/src/socket-mode/socket-mode-adapter.ts +++ b/packages/apps/src/socket-mode/socket-mode-adapter.ts @@ -23,7 +23,6 @@ import { GeoSocket } from './geo-socket'; import { NegotiateError } from './negotiate'; import { SignalRSocketConnection } from './socket-connection'; import { - DEFAULT_SOCKET_MODE_NEGOTIATE_BASE_URL, ISocketConnection, ReplyFrame, SocketActivityEnvelope, @@ -44,10 +43,11 @@ const RECONNECT_MAX_DELAY_MS = 15_000; /** Renegotiate this long before the negotiate token is due to expire. */ const TOKEN_REFRESH_MARGIN_MS = 60_000; +const SOCKET_MODE_NEGOTIATE_HOST = 'botapi.skype.com'; /** * Default geographies a bot connects to. One socket is opened per geo (the geo - * is appended as a path segment to the negotiate base), so a single bot has + * is prefixed to the negotiate hostname), so a single bot has * inbound coverage across regions rather than only the geo it happened to dial. */ const DEFAULT_GEOS = ['amer', 'emea', 'apac'] as const; @@ -62,11 +62,11 @@ function isNonRetryableError(error: unknown): boolean { return error instanceof NegotiateError && error.isAuthError; } -/** Join a base URL, an optional geo segment, and the negotiate path. */ -function buildNegotiateUrl(base: string, geo: string): string { - const trimmedBase = base.replace(/\/+$/, ''); - const seg = geo.trim().replace(/^\/+|\/+$/g, ''); - return seg ? `${trimmedBase}/${seg}${SOCKET_MODE_NEGOTIATE_PATH}` : `${trimmedBase}${SOCKET_MODE_NEGOTIATE_PATH}`; +/** + * The supplied geo is added as a subdomain before the negotiate path. + */ +function buildNegotiateUrl(geo: string): string { + return `https://${geo}.${SOCKET_MODE_NEGOTIATE_HOST}${SOCKET_MODE_NEGOTIATE_PATH}`; } /** @@ -226,20 +226,6 @@ export class SocketModeAdapter implements IHttpServerAdapter { return this.resolveGeos(); } - /** - * Resolved negotiate URL for the first geo (`{base}/{geo}/v3/websockets/connect`). - * Primarily for diagnostics; each geo has its own URL. - */ - get negotiateUrl(): string { - const geos = this.resolveGeos(); - return buildNegotiateUrl(this.negotiateBase, geos[0] ?? ''); - } - - /** Base negotiate URL (no geo segment, no path). */ - private get negotiateBase(): string { - return this.options.negotiateBaseUrl ?? DEFAULT_SOCKET_MODE_NEGOTIATE_BASE_URL; - } - /** * {@link IHttpServerAdapter} route registration. The owning {@link HttpServer} * registers its inbound-activity route here as part of normal server @@ -281,12 +267,16 @@ export class SocketModeAdapter implements IHttpServerAdapter { * it alive across drops and token expiry independently until {@link stop}. */ async start(_port?: number | string): Promise { + const geos = this.resolveGeos(); + const negotiateUrls = geos.map(buildNegotiateUrl); + this.stopped = false; this._lifecycle = 'starting'; this.abort = new AbortController(); - const geos = this.resolveGeos(); - this.geoSockets = geos.map((geo) => this.createGeoSocket(geo)); + this.geoSockets = geos.map((geo, index) => + this.createGeoSocket(geo, negotiateUrls[index]) + ); this.log.info( `socket-mode: connecting to Socket Mode across ${geos.length} geo(s): ${geos.join(', ')}` ); @@ -330,7 +320,7 @@ export class SocketModeAdapter implements IHttpServerAdapter { } /** Build one geo supervisor from the adapter's narrow internal callbacks. */ - private createGeoSocket(geo: string): GeoSocket { + private createGeoSocket(geo: string, negotiateUrl: string): GeoSocket { return new GeoSocket( { isAccepting: () => !this.stopped, @@ -350,7 +340,7 @@ export class SocketModeAdapter implements IHttpServerAdapter { tokenRefreshMarginMs: TOKEN_REFRESH_MARGIN_MS, }, geo, - buildNegotiateUrl(this.negotiateBase, geo), + negotiateUrl, this.log ); } @@ -437,12 +427,14 @@ export class SocketModeAdapter implements IHttpServerAdapter { /** The configured geos, defaulting to amer/emea/apac. */ private resolveGeos(): readonly string[] { - const geos = this.options.geos ?? DEFAULT_GEOS; - if (geos.length === 0) { - throw new Error( - 'socketMode.geos must contain at least one geo. Pass a single empty string ([""]) ' + - 'to connect to the base negotiate URL without a geo segment.' - ); + const geos = this.options.geos?.length ? this.options.geos : DEFAULT_GEOS; + geos.forEach((geo, index) => { + if (geo.length === 0) { + throw new Error(`socketMode.geos[${index}] must not be empty.`); + } + }); + if (new Set(geos.map((geo) => geo.toLowerCase())).size !== geos.length) { + throw new Error('socketMode.geos must not contain duplicates.'); } return geos; } diff --git a/packages/apps/src/socket-mode/types.spec.ts b/packages/apps/src/socket-mode/types.spec.ts index e467496eb..2ea186be3 100644 --- a/packages/apps/src/socket-mode/types.spec.ts +++ b/packages/apps/src/socket-mode/types.spec.ts @@ -1,12 +1,11 @@ import type { SocketModeOptions } from '../index'; describe('SocketModeOptions public API', () => { - it('exposes exactly the negotiate base URL and geographies', () => { + it('exposes exactly the geographies', () => { const supportedOptions: Record = { - negotiateBaseUrl: true, geos: true, }; - expect(Object.keys(supportedOptions).sort()).toEqual(['geos', 'negotiateBaseUrl']); + expect(Object.keys(supportedOptions)).toEqual(['geos']); }); }); diff --git a/packages/apps/src/socket-mode/types.ts b/packages/apps/src/socket-mode/types.ts index 0c64daef0..d1a9007e6 100644 --- a/packages/apps/src/socket-mode/types.ts +++ b/packages/apps/src/socket-mode/types.ts @@ -7,13 +7,6 @@ import { Client as HttpClient } from '@microsoft/teams.common'; */ export const SOCKET_MODE_PROTOCOL_VERSION = 1; -/** - * Default Teams backend service host used to negotiate the Socket Mode connection when no - * {@link SocketModeOptions.negotiateBaseUrl} is provided. The negotiate route - * (`/v3/websockets/connect`) is appended to this base. - */ -export const DEFAULT_SOCKET_MODE_NEGOTIATE_BASE_URL = 'https://botapi.skype.com'; - /** * The negotiate route appended to the Teams backend service base URL. Teams backend service exposes the Socket Mode * handshake at `POST {base}/v3/websockets/connect`. @@ -51,27 +44,17 @@ export type SocketModeStatus = * @experimental This API is in preview and may change in the future. */ export type SocketModeOptions = { - /** - * Base URL of the Teams backend service endpoint used to negotiate the socket. The negotiate - * request is sent to `{negotiateBaseUrl}/v3/websockets/connect`. - * - * Defaults to {@link DEFAULT_SOCKET_MODE_NEGOTIATE_BASE_URL}. Override this to - * target a different Teams backend service ring. - */ - readonly negotiateBaseUrl?: string; - /** * Geographies to connect to. One independent socket is opened per geo, with - * the geo appended as a path segment to the negotiate base - * (`{negotiateBaseUrl}/{geo}/v3/websockets/connect`), so a single bot has + * the geo prefixed to the SDK-owned negotiate hostname + * (`https://{geo}.botapi.skype.com/v3/websockets/connect`), so a single bot has * inbound coverage across regions — a conversation pinned to another geo is * still delivered over the socket instead of silently falling back to HTTP. * - * Defaults to `['amer', 'emea', 'apac']`. Every listed geo must connect for - * `App.start()` to succeed; each is then supervised independently, so one geo - * dropping never affects delivery on the others. Pass a single empty string - * (`['']`) to connect to the base negotiate URL with no geo segment. Must be - * non-empty. + * Omitted or empty lists default to `['amer', 'emea', 'apac']`. Every listed + * geo must connect for `App.start()` to succeed; each is then supervised + * independently, so one geo dropping never affects delivery on the others. + * Geo values must be non-empty and case-insensitively unique. * * @default ['amer', 'emea', 'apac'] */ @@ -236,7 +219,7 @@ export type SocketConnectionHandlers = { * connection. */ export type SocketConnectionContext = { - /** Resolved negotiate URL (`{negotiateBaseUrl}/v3/websockets/connect`). */ + /** Resolved regional negotiate URL. */ readonly negotiateUrl: string; /** The app's shared HTTP client used for the negotiate request. */ readonly client: HttpClient; diff --git a/version.json b/version.json index 1eeba5c15..873c16286 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/main/src/NerdBank.GitVersioning/version.schema.json", - "version": "2.2.0", + "version": "2.2.1", "publicReleaseRefSpec": [ "^refs/heads/main$", "^refs/heads/release/v\\d+\\.\\d+$"