Skip to content

Commit

Permalink
fix(client): fix websocket client protocol (#2479)
Browse files Browse the repository at this point in the history
* chore(client): create replaceUrlProtocol

* chore(client): fix websocket client url protocol bug

* chore(deno_dist): generate deno code

* chore(client): add test

* chore(client): change test case description

* chore(client): add https, wss test
  • Loading branch information
naporin0624 committed Apr 9, 2024
1 parent b0d9417 commit 1d4c67a
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 7 deletions.
15 changes: 12 additions & 3 deletions deno_dist/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@ import type { ValidationTargets } from '../types.ts'
import { serialize } from '../utils/cookie.ts'
import type { UnionToIntersection } from '../utils/types.ts'
import type { Callback, Client, ClientRequestOptions } from './types.ts'
import { deepMerge, mergePath, removeIndexString, replaceUrlParam } from './utils.ts'
import {
deepMerge,
mergePath,
removeIndexString,
replaceUrlParam,
replaceUrlProtocol,
} from './utils.ts'

const createProxy = (callback: Callback, path: string[]) => {
const proxy: unknown = new Proxy(() => {}, {
Expand Down Expand Up @@ -147,8 +153,11 @@ export const hc = <T extends Hono<any, any, any>>(
return new URL(url)
}
if (method === 'ws') {
const targetUrl =
opts.args[0] && opts.args[0].param ? replaceUrlParam(url, opts.args[0].param) : url
const targetUrl = replaceUrlProtocol(
opts.args[0] && opts.args[0].param ? replaceUrlParam(url, opts.args[0].param) : url,
'ws'
)

return new WebSocket(targetUrl)
}

Expand Down
9 changes: 9 additions & 0 deletions deno_dist/client/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ export const replaceUrlParam = (urlString: string, params: Record<string, string
return urlString
}

export const replaceUrlProtocol = (urlString: string, protocol: 'ws' | 'http') => {
switch (protocol) {
case 'ws':
return urlString.replace(/^http/, 'ws')
case 'http':
return urlString.replace(/^ws/, 'http')
}
}

export const removeIndexString = (urlSting: string) => {
return urlSting.replace(/\/index$/, '')
}
Expand Down
56 changes: 56 additions & 0 deletions src/client/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { rest } from 'msw'
import { setupServer } from 'msw/node'
import { expectTypeOf, vi } from 'vitest'
import { upgradeWebSocket } from '../helper'
import { Hono } from '../hono'
import { parse } from '../utils/cookie'
import type { Equal, Expect } from '../utils/types'
Expand Down Expand Up @@ -686,3 +687,58 @@ describe('Dynamic headers', () => {
expect(data.requestDynamic).toEqual('two')
})
})

describe('WebSocket URL Protocol Translation', () => {
const app = new Hono()
const route = app.get(
'/',
upgradeWebSocket((c) => ({
onMessage(event, ws) {
console.log(`Message from client: ${event.data}`)
ws.send('Hello from server!')
},
onClose: () => {
console.log('Connection closed')
},
}))
)

type AppType = typeof route

const server = setupServer()
const webSocketMock = vi.fn()

beforeAll(() => server.listen())
beforeEach(() => {
vi.stubGlobal('WebSocket', webSocketMock)
})
afterEach(() => {
vi.clearAllMocks()
server.resetHandlers()
})
afterAll(() => server.close())

it('Translates HTTP to ws', async () => {
const client = hc<AppType>('http://localhost')
client.index.$ws()
expect(webSocketMock).toHaveBeenCalledWith('ws://localhost/index')
})

it('Translates HTTPS to wss', async () => {
const client = hc<AppType>('https://localhost')
client.index.$ws()
expect(webSocketMock).toHaveBeenCalledWith('wss://localhost/index')
})

it('Keeps ws unchanged', async () => {
const client = hc<AppType>('ws://localhost')
client.index.$ws()
expect(webSocketMock).toHaveBeenCalledWith('ws://localhost/index')
})

it('Keeps wss unchanged', async () => {
const client = hc<AppType>('wss://localhost')
client.index.$ws()
expect(webSocketMock).toHaveBeenCalledWith('wss://localhost/index')
})
})
15 changes: 12 additions & 3 deletions src/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@ import type { ValidationTargets } from '../types'
import { serialize } from '../utils/cookie'
import type { UnionToIntersection } from '../utils/types'
import type { Callback, Client, ClientRequestOptions } from './types'
import { deepMerge, mergePath, removeIndexString, replaceUrlParam } from './utils'
import {
deepMerge,
mergePath,
removeIndexString,
replaceUrlParam,
replaceUrlProtocol,
} from './utils'

const createProxy = (callback: Callback, path: string[]) => {
const proxy: unknown = new Proxy(() => {}, {
Expand Down Expand Up @@ -147,8 +153,11 @@ export const hc = <T extends Hono<any, any, any>>(
return new URL(url)
}
if (method === 'ws') {
const targetUrl =
opts.args[0] && opts.args[0].param ? replaceUrlParam(url, opts.args[0].param) : url
const targetUrl = replaceUrlProtocol(
opts.args[0] && opts.args[0].param ? replaceUrlParam(url, opts.args[0].param) : url,
'ws'
)

return new WebSocket(targetUrl)
}

Expand Down
34 changes: 33 additions & 1 deletion src/client/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { deepMerge, mergePath, removeIndexString, replaceUrlParam } from './utils'
import {
deepMerge,
mergePath,
removeIndexString,
replaceUrlParam,
replaceUrlProtocol,
} from './utils'

describe('mergePath', () => {
it('Should merge paths correctly', () => {
Expand Down Expand Up @@ -42,6 +48,32 @@ describe('replaceUrlParams', () => {
})
})

describe('replaceUrlProtocol', () => {
it('Should replace http to ws', () => {
const url = 'http://localhost'
const newUrl = replaceUrlProtocol(url, 'ws')
expect(newUrl).toBe('ws://localhost')
})

it('Should replace https to wss', () => {
const url = 'https://localhost'
const newUrl = replaceUrlProtocol(url, 'ws')
expect(newUrl).toBe('wss://localhost')
})

it('Should replace ws to http', () => {
const url = 'ws://localhost'
const newUrl = replaceUrlProtocol(url, 'http')
expect(newUrl).toBe('http://localhost')
})

it('Should replace wss to https', () => {
const url = 'wss://localhost'
const newUrl = replaceUrlProtocol(url, 'http')
expect(newUrl).toBe('https://localhost')
})
})

describe('removeIndexString', () => {
it('Should remove last `/index` string', () => {
let url = 'http://localhost/index'
Expand Down
9 changes: 9 additions & 0 deletions src/client/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ export const replaceUrlParam = (urlString: string, params: Record<string, string
return urlString
}

export const replaceUrlProtocol = (urlString: string, protocol: 'ws' | 'http') => {
switch (protocol) {
case 'ws':
return urlString.replace(/^http/, 'ws')
case 'http':
return urlString.replace(/^ws/, 'http')
}
}

export const removeIndexString = (urlSting: string) => {
return urlSting.replace(/\/index$/, '')
}
Expand Down

0 comments on commit 1d4c67a

Please sign in to comment.