From 1765a9a3aa1ab6aa59b0efd2d161b7bfaa565ef7 Mon Sep 17 00:00:00 2001
From: Shotaro Nakamura <79000684+nakasyou@users.noreply.github.com>
Date: Sun, 27 Apr 2025 20:34:43 +0900
Subject: [PATCH] fix(node-ws): make adapter uncrashable (#1141)

* fix(node-ws): make adapter uncrashable

* add changeset

* unnessesary diff

* update yarn.lock

* make changeset patch
---
 .changeset/nice-rice-agree.md      |  5 +++
 packages/node-ws/src/index.test.ts | 20 +++++++++
 packages/node-ws/src/index.ts      | 72 +++++++++++++++++++++---------
 3 files changed, 75 insertions(+), 22 deletions(-)
 create mode 100644 .changeset/nice-rice-agree.md

diff --git a/.changeset/nice-rice-agree.md b/.changeset/nice-rice-agree.md
new file mode 100644
index 00000000..0c2fd0aa
--- /dev/null
+++ b/.changeset/nice-rice-agree.md
@@ -0,0 +1,5 @@
+---
+'@hono/node-ws': patch
+---
+
+Make it uncrashable
diff --git a/packages/node-ws/src/index.test.ts b/packages/node-ws/src/index.test.ts
index c7d6d212..9815e4fe 100644
--- a/packages/node-ws/src/index.test.ts
+++ b/packages/node-ws/src/index.test.ts
@@ -214,6 +214,26 @@ describe('WebSocket helper', () => {
     })
   })
 
+  it('Should onError works well', async () => {
+    const mainPromise = new Promise<unknown>((resolve) =>
+      app.get(
+        '/',
+        upgradeWebSocket(
+          () => {
+            throw 0
+          },
+          {
+            onError(err) {
+              resolve(err)
+            },
+          }
+        )
+      )
+    )
+    const ws = new WebSocket('ws://localhost:3030/')
+    expect(await mainPromise).toBe(0)
+  })
+
   describe('Types', () => {
     it('Should not throw a type error with an app with Variables generics', () => {
       const app = new Hono<{
diff --git a/packages/node-ws/src/index.ts b/packages/node-ws/src/index.ts
index 60aa14f0..ed0c5c79 100644
--- a/packages/node-ws/src/index.ts
+++ b/packages/node-ws/src/index.ts
@@ -1,5 +1,5 @@
 import type { Hono } from 'hono'
-import type { UpgradeWebSocket, WSContext } from 'hono/ws'
+import type { UpgradeWebSocket, WSContext, WSEvents } from 'hono/ws'
 import type { WebSocket } from 'ws'
 import { WebSocketServer } from 'ws'
 import type { IncomingMessage } from 'http'
@@ -9,7 +9,12 @@ import type { Duplex } from 'node:stream'
 import { CloseEvent } from './events'
 
 export interface NodeWebSocket {
-  upgradeWebSocket: UpgradeWebSocket<WebSocket>
+  upgradeWebSocket: UpgradeWebSocket<
+    WebSocket,
+    {
+      onError: (err: unknown) => void
+    }
+  >
   injectWebSocket(server: Server | Http2Server | Http2SecureServer): void
 }
 export interface NodeWebSocketInit {
@@ -80,7 +85,7 @@ export const createNodeWebSocket = (init: NodeWebSocketInit): NodeWebSocket => {
         })
       })
     },
-    upgradeWebSocket: (createEvents) =>
+    upgradeWebSocket: (createEvents, options) =>
       async function upgradeWebSocket(c, next) {
         if (c.req.header('upgrade')?.toLowerCase() !== 'websocket') {
           // Not websocket
@@ -91,7 +96,14 @@ export const createNodeWebSocket = (init: NodeWebSocketInit): NodeWebSocket => {
         const response = new Response()
         ;(async () => {
           const ws = await nodeUpgradeWebSocket(c.env.incoming, response)
-          const events = await createEvents(c)
+          let events: WSEvents<WebSocket>
+          try {
+            events = await createEvents(c)
+          } catch (e) {
+            ;(options?.onError ?? console.error)(e)
+            ws.close()
+            return
+          }
 
           const ctx: WSContext<WebSocket> = {
             binaryType: 'arraybuffer',
@@ -110,32 +122,48 @@ export const createNodeWebSocket = (init: NodeWebSocketInit): NodeWebSocket => {
             },
             url: new URL(c.req.url),
           }
-          events.onOpen?.(new Event('open'), ctx)
+          try {
+            events?.onOpen?.(new Event('open'), ctx)
+          } catch (e) {
+            ;(options?.onError ?? console.error)(e)
+          }
           ws.on('message', (data, isBinary) => {
             const datas = Array.isArray(data) ? data : [data]
             for (const data of datas) {
-              events.onMessage?.(
-                new MessageEvent('message', {
-                  data: isBinary
-                    ? data instanceof ArrayBuffer
-                      ? data
-                      : data.buffer.slice(data.byteOffset, data.byteOffset + data.byteLength)
-                    : data.toString('utf-8'),
-                }),
-                ctx
-              )
+              try {
+                events?.onMessage?.(
+                  new MessageEvent('message', {
+                    data: isBinary
+                      ? data instanceof ArrayBuffer
+                        ? data
+                        : data.buffer.slice(data.byteOffset, data.byteOffset + data.byteLength)
+                      : data.toString('utf-8'),
+                  }),
+                  ctx
+                )
+              } catch (e) {
+                ;(options?.onError ?? console.error)(e)
+              }
             }
           })
           ws.on('close', (code, reason) => {
-            events.onClose?.(new CloseEvent('close', { code, reason: reason.toString() }), ctx)
+            try {
+              events?.onClose?.(new CloseEvent('close', { code, reason: reason.toString() }), ctx)
+            } catch (e) {
+              ;(options?.onError ?? console.error)(e)
+            }
           })
           ws.on('error', (error) => {
-            events.onError?.(
-              new ErrorEvent('error', {
-                error: error,
-              }),
-              ctx
-            )
+            try {
+              events?.onError?.(
+                new ErrorEvent('error', {
+                  error: error,
+                }),
+                ctx
+              )
+            } catch (e) {
+              ;(options?.onError ?? console.error)(e)
+            }
           })
         })()