feat: next() without await

[yusukebe]

I realized that we don't have to write await for next() in middleware. This means if it does not have async functions in middleware, you can write like the following:

app.get('*', (c, next) => {
  console.log('this is middleware')
  next()
  c.res.headers.append('foo', 'bar')
})

If so, this improves performance when using middleware. Using await will be slow with many middlewares.

I've taken a benchmark. The code is here:

import { Hono } from 'hono'
import { createMiddleware } from 'hono/factory'

const app = new Hono()

const mwWithAwait = createMiddleware(async (c, next) => {
  const foo = 'foo'
  await next()
})

const mwWithoutAwait = createMiddleware((c, next) => {
  const foo = 'foo'
  next()
})

app.get('/', (c) => c.text('hi'))
app.get('/mwWithAwait', mwWithAwait, mwWithAwait, mwWithAwait, mwWithAwait, mwWithAwait, (c) => c.text('hi'))
app.get('/mwWithoutAwait', mwWithoutAwait, mwWithoutAwait, mwWithoutAwait, mwWithoutAwait, mwWithoutAwait, (c) =>
  c.text('hi')
)

And the result:

We can use next() without await, we just haven't done it that way until now.

In this PR, I've added tests for without await and fixed the type definitions.

Author should do the followings, if applicable

• Add tests
• Run tests
• yarn denoify to generate files for Deno

[yusukebe]

Hi @usualoma !

What do you think about this? Can we really use it without await?

[usualoma]

Hi @yusukebe!

Hmmm, I'm sure there are apps that are synchronous and have no problem working without await, but if the handlers were asynchronous, I think all middleware would have to await next() with the current design. Therefore, I don't think it is possible to make it so that you don't have to await unless we change the design very drastically.

The following application will result in an Internal Server Error.

import { createMiddleware } from './src/helper/factory'
import { Hono } from './src/hono'

const app = new Hono()

app.get(
  '/asyncContent',
  createMiddleware((c, next) => {
    const foo = 'foo'
    next()
  }),
  async (c) => {
    const data = await (await fetch('https://ramen-api.dev/shops/takasagoya')).json()
    return c.json(data)
  }
)

export default app

Or, as shown below, even if the outer middleware uses await, if middleware that does not use await is inserted in between, it will not work properly.
This makes debugging difficult.

import { createMiddleware } from './src/helper/factory'
import { Hono } from './src/hono'

const app = new Hono()

app.get(
  '/asyncContent',
  createMiddleware(async (c, next) => {
    await next()
    c.res = new Response('asyncContent ' + await c.res.text()) // rewrite response
  }),
  createMiddleware((c, next) => {
    next()
  }),
  async (c) => {
    const data = await (await fetch('https://ramen-api.dev/shops/takasagoya')).json()
    return c.json(data)
  }
)

export default app

[yusukebe]

Hmm, you're right. Any ideas... I want to resolve this issue of increasing overhead when middleware is added. I think we can change the API just for this, and we could release it as v4.

[yusukebe]

This is just an idea. I don't know how to implement it yet, but we propose syntax like this, though next might be a more appropriate name.

app.get(
  '/asyncContent',
  // POC
  // You don't have to `await next()`
  createMiddleware((c, next) => {
    console.log('mw - before handler')
    next(() => {
      console.log('mw - after handler')
    })
  }),
  async (c) => {
    console.log('handler')
    const data = await (await fetch('https://ramen-api.dev/shops/takasagoya')).json()
    return c.json(data)
  }
)

[usualoma]

If we could just avoid async / await, I think we could write the following in the current spec.
As far as I tested it on my environment with Bun, the performance did not change.

However, if we want to do something with the middleware after the next() operation, the middleware must return a Promise anyway, or else the dispatch will be over before the internal async is resolved.

app.get(
  '/asyncContent',
  createMiddleware((c, next) => {
    return next().then(() => {
      c.res.text().then((text) => {
        c.res = new Response('asyncContent ' + text) // rewrite response
      })
    })
  }),
  async (c) => {
    const data = await (await fetch('https://ramen-api.dev/shops/takasagoya')).json()
    return c.json(data)
  }
)

If you really want to reduce async / await and return next().then() calls, I think we can do the following.
#1689

With this PR, we can add middleware that is "synchronous and does not call next()" while maintaining high performance.

import { createMiddleware } from './src/helper/factory'
import { Hono } from './src/hono'

const app = new Hono()

app.get(
  '/asyncContent/:name',
  ...Array.from(Array(10)).map((_, i) =>
    createMiddleware('before', (c) => {
      c.res.headers.set('overwrite-by-handler', 'by middleware')
      c.res.headers.set(`Before-${i}`, 'Hono')
    })
  ),
  ...Array.from(Array(10)).map((_, i) =>
    createMiddleware('after', (c) => {
      c.res.headers.set('not-overwrite-by-handler', 'by middleware')
      c.res.headers.set(`After-${i}`, 'Hono')
    })
  ),
  async (c) => {
    c.res.headers.set('overwrite-by-handler', 'by handler')
    c.res.headers.set('not-overwrite-by-handler', 'by handler')
    const data = await (await fetch(`https://ramen-api.dev/shops/${c.req.param('name')}`)).json()
    return c.json(data)
  }
)

export default app

% curl -i http://localhost:3000/asyncContent/takasagoya
HTTP/1.1 200 OK
Content-Type: application/json; charset=UTF-8
before-0: Hono
before-1: Hono
before-2: Hono
before-3: Hono
before-4: Hono
before-5: Hono
before-6: Hono
before-7: Hono
before-8: Hono
before-9: Hono
not-overwrite-by-handler: by middleware
overwrite-by-handler: by handler
After-0: Hono
After-1: Hono
After-2: Hono
After-3: Hono
After-4: Hono
After-5: Hono
After-6: Hono
After-7: Hono
After-8: Hono
After-9: Hono
Date: Sun, 12 Nov 2023 23:39:48 GMT
Content-Length: 209

{"shop":{"id":"takasagoya","name":"たかさご家","photos":[{"name":"takasagoya-001.jpg","width":1200,"height":900,"authorId":"yusukebe","url":"https://ramen-api.dev/images/takasagoya/takasagoya-001.jpg"}]}}

[usualoma]

I think #1689 is an interesting idea, but I am not sure that it is optimized enough to be meaningful in a production environment beyond producing good results in benchmark scripts.
The code will be complex and maintenance costs will be high.

[yusukebe]

Hi @usualoma,

Thanks! That's interesting. I'm also struggling to find a good solution. Let's take some time to work on it.

[yusukebe]

@usualoma

How do you feel about this API, although it has not been implemented yet?

const mwWithAwait = createMiddleware(async (c, next) => {
  before()
  await next()
  after()
})

const mwWithoutAwait = createMiddleware((c, next) => {
  before()
  return next(after())
})

[usualoma]

Yes, I think such a change is possible.
I think we can accomplish that with the following changes.

diff --git a/src/compose.ts b/src/compose.ts
index 56a755b..bf1491f 100644
--- a/src/compose.ts
+++ b/src/compose.ts
@@ -42,9 +42,13 @@ export const compose = <C extends ComposeContext, E extends Env = Env>(
         }
       } else {
         try {
-          res = handler(context, () => {
+          res = handler(context, (after?: () => void | Promise<void>) => {
             const dispatchRes = dispatch(i + 1)
-            return dispatchRes instanceof Promise ? dispatchRes : Promise.resolve(dispatchRes)
+            return !after
+              ? dispatchRes
+              : dispatchRes instanceof Promise
+              ? dispatchRes.then(after)
+              : after()
           })
         } catch (err) {
           if (err instanceof Error && context instanceof Context && onError) {
diff --git a/src/types.ts b/src/types.ts
index 0c031ac..7bec387 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -20,7 +20,7 @@ export type Env = {
   Variables?: Variables
 }
 
-export type Next = () => void | Promise<void>
+export type Next = (after?: () => void | Promise<void>) => void | Promise<void>
 
 export type Input = {
   in?: Partial<ValidationTargets>

At this time, the following apps will "/mwWithoutAwait" faster

import { createMiddleware } from './src/helper/factory'
import { Hono } from './src/hono'

const app = new Hono()

const mwWithAwait = createMiddleware(async (c, next) => {
  const foo = 'foo'
  await next()
  const bar = 'bar'
})

const mwWithoutAwait = createMiddleware((c, next) => {
  const foo = 'foo'
  return next(() => {
    const bar = 'bar'
  })
})

app.get('/', (c) => c.text('hi'))
app.get('/mwWithAwait', mwWithAwait, mwWithAwait, mwWithAwait, mwWithAwait, mwWithAwait, (c) => c.text('hi'))
app.get('/mwWithoutAwait', mwWithoutAwait, mwWithoutAwait, mwWithoutAwait, mwWithoutAwait, mwWithoutAwait, (c) =>
  c.text('hi')
)

export default app

% bombardier -d 10s --fasthttp http://localhost:3000/mwWithAwait
Bombarding http://localhost:3000/mwWithAwait for 10s using 125 connection(s)
[==========================================================================] 10s
Done!
Statistics        Avg      Stdev        Max
  Reqs/sec     44575.86    2459.14   49854.11
  Latency        2.80ms   302.86us    20.06ms
  HTTP codes:
    1xx - 0, 2xx - 445615, 3xx - 0, 4xx - 0, 5xx - 0
    others - 0
  Throughput:     8.08MB/s
% bombardier -d 10s --fasthttp http://localhost:3000/mwWithoutAwait
Bombarding http://localhost:3000/mwWithoutAwait for 10s using 125 connection(s)
[==========================================================================] 10s
Done!
Statistics        Avg      Stdev        Max
  Reqs/sec     52860.92    7915.00   92363.19
  Latency        2.36ms   278.29us    20.95ms
  HTTP codes:
    1xx - 0, 2xx - 528474, 3xx - 0, 4xx - 0, 5xx - 0
    others - 0
  Throughput:     9.73MB/s

However, if a Promise is returned from the handler, the difference is much smaller, although '/mwWithoutAwait is faster.

import { createMiddleware } from './src/helper/factory'
import { Hono } from './src/hono'

const app = new Hono()

const mwWithAwait = createMiddleware(async (c, next) => {
  const foo = 'foo'
  await next()
  const bar = 'bar'
})

const mwWithoutAwait = createMiddleware((c, next) => {
  const foo = 'foo'
  return next(() => {
    const bar = 'bar'
  })
})

app.get('/', (c) => c.text('hi'))
app.get('/mwWithAwait', mwWithAwait, mwWithAwait, mwWithAwait, mwWithAwait, mwWithAwait, (c) => c.text('hi'))
app.get('/mwWithoutAwait', mwWithoutAwait, mwWithoutAwait, mwWithoutAwait, mwWithoutAwait, mwWithoutAwait, (c) =>
  Promise.resolve(c.text('hi'))
)

export default app

% bombardier -d 10s --fasthttp http://localhost:3000/mwWithAwait
Bombarding http://localhost:3000/mwWithAwait for 10s using 125 connection(s)
[==========================================================================] 10s
Done!
Statistics        Avg      Stdev        Max
  Reqs/sec     43961.59    2534.22   48521.80
  Latency        2.84ms   401.92us    26.80ms
  HTTP codes:
    1xx - 0, 2xx - 439404, 3xx - 0, 4xx - 0, 5xx - 0
    others - 0
  Throughput:     7.96MB/s
% bombardier -d 10s --fasthttp http://localhost:3000/mwWithoutAwait
Bombarding http://localhost:3000/mwWithoutAwait for 10s using 125 connection(s)
[==========================================================================] 10s
Done!
Statistics        Avg      Stdev        Max
  Reqs/sec     44589.03    3487.45   49663.37
  Latency        2.80ms   454.29us    28.75ms
  HTTP codes:
    1xx - 0, 2xx - 445567, 3xx - 0, 4xx - 0, 5xx - 0
    others - 0
  Throughput:     8.20MB/s

I believe this change will optimize performance only if all of the handlers and middleware are processed synchronously. In a production environment, most handlers will work asynchronously, so I think the optimization will be applied only in a few cases in a production environment.

I think there will be a significant difference on the benchmark script.

It would also be good if the official middleware is rewritten to support this.
However, while "powered-by" can easily support this specification, "basic-auth" and many others require asynchronous processing, and I think there are few middleware that can support it.

hono/src/middleware/basic-auth/index.ts

Lines 46 to 50 in 864fae6

	const usernameEqual = await timingSafeEqual(
	user.username,
	requestUser.username,
	options.hashFunction
	)

On the other hand, I think it is better to continue to encourage Hono users to write await next(). It is simpler and easier to understand than return next(after).

Also, with the above change, the following code will not produce a type error (the current main branch will produce an error) I think the inability to produce type errors is a major problem, so I think we will have to devise a solution (a rather difficult problem).

const middlewareThatCausesProblemsInAsyncResult = createMiddleware((c, next) => {
  const foo = 'foo'
  next()
  const bar = 'bar'
})

[yusukebe]

@usualoma

Thanks!

On the other hand, I think it is better to continue to encourage Hono users to write await next(). It is simpler and easier to understand than return next(after).

I agree with this too.

I've also refactored context.ts, making it much simpler. With these changes, in my environment, middleware that uses await next() seems to be faster.

diff --git a/src/compose.ts b/src/compose.ts
index 56a755b..b375dcd 100644
--- a/src/compose.ts
+++ b/src/compose.ts
@@ -17,7 +17,7 @@ export const compose = <C extends ComposeContext, E extends Env = Env>(
     let index = -1
     return dispatch(0)

-    function dispatch(i: number): C | Promise<C> {
+    async function dispatch(i: number): Promise<C> {
       if (i <= index) {
         throw new Error('next() called multiple times')
       }
@@ -42,9 +42,8 @@ export const compose = <C extends ComposeContext, E extends Env = Env>(
         }
       } else {
         try {
-          res = handler(context, () => {
-            const dispatchRes = dispatch(i + 1)
-            return dispatchRes instanceof Promise ? dispatchRes : Promise.resolve(dispatchRes)
+          res = await handler(context, () => {
+            return dispatch(i + 1)
           })
         } catch (err) {
           if (err instanceof Error and context instanceof Context and onError) {
@@ -57,34 +56,13 @@ export const compose = <C extends ComposeContext, E extends Env = Env>(
         }
       }

-      if (!(res instanceof Promise)) {
-        if (res !== undefined and 'response' in res) {
-          res = res['response']
-        }
-        if (res and (context.finalized === false or isError)) {
-          context.res = res
-        }
-        return context
-      } else {
-        return res
-          .then((res) => {
-            if (res !== undefined and 'response' in res) {
-              res = res['response']
-            }
-            if (res and context.finalized === false) {
-              context.res = res
-            }
-            return context
-          })
-          .catch(async (err) => {
-            if (err instanceof Error and context instanceof Context and onError) {
-              context.error = err
-              context.res = await onError(err, context)
-              return context
-            }
-            throw err
-          })
+      if (res !== undefined and 'response' in res) {
+        res = res['response']
+      }
+      if (res and (context.finalized === false or isError)) {
+        context.res = res
       }
+      return context
     }
   }
 }

We need to benchmark again to be sure, but if this improves performance, it would be a good change.

[usualoma]

Hi @yusukebe!

The changes you have indicated here are good ones! It will reduce the amount of code and clarify the intent.
#1685 (comment)

However, with this change, the following code would have the same level of performance. (It may depend on the runtime environment, though. I did not see any difference when I tried it on Bun).

const mwWithAwait = createMiddleware(async (c, next) => {
  before()
  await next()
  after()
})

const mwWithoutAwait = createMiddleware((c, next) => {
  before()
  return next(after)
})

[yusukebe]

However, with this change, the following code would have the same level of performance.

Ah, sorry for the confusion! I meant that using only await in middleware is slightly faster.

[usualoma]

@yusukebe
Ah yes, sorry I didn't mention it; I understood the point about "improved performance when using await."
So, I fully agree with you about the #1685 (comment) patch.

[metrue]

I haven't been actively contributing to Hono for a while due to personal life changes. I've been periodically checking the issues in the repository and stumbled upon this discussion.
I'm particularly curious about the extent of performance improvement we can achieve by eliminating async from the middleware. I know that @yusukebe conducted some benchmarks on this, but the observed improvement doesn't seem significant to me.
I'm also interested in understanding the impact on users after removing async from the middleware. Is it purely an internal change, or will it affect how users can enable the middleware in their projects? While I've come across some insightful discussions (https://v8.dev/blog/fast-async, nodejs/node#31979) on threads related to async and promise, there hasn't been a strong suggestion that using async significantly harms performance. However, I acknowledge the need to delve deeper into this topic to enhance my understanding.

[yusukebe]

Hi @metrue !

This is interesting. The facts that we/me and @usualoma discussed is mostly about running apps on Bun. For Bun, await/async makes it slower compared to Node.js.

On Bun, there is a difference between using await() and not using await():

But, on Node.js. It's a very few.

Recently, we've been always measuring benchmark scores on Bun. As a result, we pay attention to the use of await/async. However, we don't have to focus on this when the user runs on Node.js. This difference might be due to whether the JS engine is V8 or JavaScriptCore. The same considerations for Node.js apply to Deno and Cloudflare Workers. So, perhaps I'm overthinking things.

[mharj]

Promise itself is just two callbacks for handle things async, I kind of would disagree with change to callback. Promise is very clever (because can be ref) than doing manual callbacks. As example, if I have middleware which is async by nature (singleshot promise) .. now two Requests incoming and if this have callback setup, it means that need to wrap/run extra promises for each request (await before callback()) as we can't anymore pass actual middleware promise as next(). So even though this might give some edge case performance you might acually consume more memory to just avoid await resolve callback as instead have one ref you will have many. As overy simplfied example .. think that middleware chain is just static promise chain, do you like to refer to this single Promise callback for this chain or cause one extra await Promise resolve() which runs before drop to callback()