fix(hono-base): fix not found handler calling timing when one middlew…

…are (#2080)
honojs · Jan 25, 2024 · 4a239ad · 4a239ad
1 parent 27521e6
commit 4a239ad
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 32 deletions.
diff --git a/deno_dist/hono-base.ts b/deno_dist/hono-base.ts
@@ -308,28 +308,21 @@ class Hono<
     if (matchResult[0].length === 1) {
       let res: ReturnType<H>
       try {
-        res = matchResult[0][0][0][0](c, async () => {})
-        if (!res) {
-          return this.notFoundHandler(c)
-        }
+        res = matchResult[0][0][0][0](c, async () => {
+          c.res = await this.notFoundHandler(c)
+        })
       } catch (err) {
         return this.handleError(err, c)
       }
 
       if (res instanceof Response) return res
 
-      return (async () => {
-        let awaited: Response | void
-        try {
-          awaited = await res
-          if (!awaited) {
-            return this.notFoundHandler(c)
-          }
-        } catch (err) {
-          return this.handleError(err, c)
-        }
-        return awaited
-      })()
+      return res
+        .then(
+          (resolved: Response | undefined) =>
+            resolved || (c.finalized ? c.res : this.notFoundHandler(c))
+        )
+        .catch((err: Error) => this.handleError(err, c))
     }
 
     const composed = compose<Context>(matchResult[0], this.errorHandler, this.notFoundHandler)

diff --git a/src/hono-base.ts b/src/hono-base.ts
@@ -308,28 +308,21 @@ class Hono<
     if (matchResult[0].length === 1) {
       let res: ReturnType<H>
       try {
-        res = matchResult[0][0][0][0](c, async () => {})
-        if (!res) {
-          return this.notFoundHandler(c)
-        }
+        res = matchResult[0][0][0][0](c, async () => {
+          c.res = await this.notFoundHandler(c)
+        })
       } catch (err) {
         return this.handleError(err, c)
       }
 
       if (res instanceof Response) return res
 
-      return (async () => {
-        let awaited: Response | void
-        try {
-          awaited = await res
-          if (!awaited) {
-            return this.notFoundHandler(c)
-          }
-        } catch (err) {
-          return this.handleError(err, c)
-        }
-        return awaited
-      })()
+      return res
+        .then(
+          (resolved: Response | undefined) =>
+            resolved || (c.finalized ? c.res : this.notFoundHandler(c))
+        )
+        .catch((err: Error) => this.handleError(err, c))
     }
 
     const composed = compose<Context>(matchResult[0], this.errorHandler, this.notFoundHandler)

diff --git a/src/hono.test.ts b/src/hono.test.ts
@@ -1051,6 +1051,64 @@ describe('Not Found', () => {
     expect(res.status).toBe(404)
     expect(await res.text()).toBe('Custom 404 Not Found')
   })
+
+  describe('Not Found with a middleware', () => {
+    const app = new Hono()
+
+    app.get('/', (c) => c.text('hello'))
+    app.use('*', async (c, next) => {
+      await next()
+      c.res = new Response((await c.res.text()) + ' + Middleware', c.res)
+    })
+
+    it('Custom 404 Not Found', async () => {
+      let res = await app.request('http://localhost/')
+      expect(res.status).toBe(200)
+      expect(await res.text()).toBe('hello')
+      res = await app.request('http://localhost/foo')
+      expect(res.status).toBe(404)
+      expect(await res.text()).toBe('404 Not Found + Middleware')
+    })
+  })
+
+  describe('Not Found with some middleware', () => {
+    const app = new Hono()
+
+    app.get('/', (c) => c.text('hello'))
+    app.use('*', async (c, next) => {
+      await next()
+      c.res = new Response((await c.res.text()) + ' + Middleware 1', c.res)
+    })
+    app.use('*', async (c, next) => {
+      await next()
+      c.res = new Response((await c.res.text()) + ' + Middleware 2', c.res)
+    })
+
+    it('Custom 404 Not Found', async () => {
+      let res = await app.request('http://localhost/')
+      expect(res.status).toBe(200)
+      expect(await res.text()).toBe('hello')
+      res = await app.request('http://localhost/foo')
+      expect(res.status).toBe(404)
+      expect(await res.text()).toBe('404 Not Found + Middleware 2 + Middleware 1')
+    })
+  })
+
+  describe('No response from a handler', () => {
+    const app = new Hono()
+
+    app.get('/', (c) => c.text('hello'))
+    app.get('/not-found', async (c) => undefined)
+
+    it('Custom 404 Not Found', async () => {
+      let res = await app.request('http://localhost/')
+      expect(res.status).toBe(200)
+      expect(await res.text()).toBe('hello')
+      res = await app.request('http://localhost/not-found')
+      expect(res.status).toBe(404)
+      expect(await res.text()).toBe('404 Not Found')
+    })
+  })
 })
 
 describe('Redirect', () => {