honojs / hono

Web framework built on Web Standards
https://hono.dev
MIT License
18.73k stars 531 forks source link

executionCtx getter throws on Bun #2649

Open comunidadio opened 4 months ago

comunidadio commented 4 months ago

What version of Hono are you using?

4.3.3

What runtime/platform is your app running on?

Bun

What steps can reproduce the bug?

I want to use executionCtx.waitUntil on CloudFlare Workers, but ignore and just await when running on Bun (since waitUntil is unsupported by Bun.serve)

app.get('/example', async (c) => {
    const p = getSomePromise();
    if (c.executionCtx) {
       c.executionCtx.waitUntil(p);
    } else {
        await p;
    }
    return c.text("OK!")
})

However this crashes at the "if (c.executionCtx)" guard because executionCtx is a getter function that throws when it's not available.

Shouldn't this return undefined or null instead of throwing to allow for the use case above?

What is the expected behavior?

Do not throw.

What do you see instead?

error: This context has no ExecutionContext as per https://github.com/honojs/hono/blob/aebaa2846228eea30a1787cde01a3d2e25fc803f/src/context.ts#L183

Additional information

No response

yusukebe commented 4 months ago

Hi @comunidadio !

Thank you for creating the issue. It's not a bug, but we may have to consider whether it should throw the error or not. As you said, it's convenient that it returns undefined.

It may not be the best solution, but you can use getRuntimeKey() to detect the runtime and make it do the c.executionCxt only in the workerd case.

app.get('/example', async (c) => {
  const p = getSomePromise()
  if (getRuntimeKey() === 'workerd') {
    c.executionCtx.waitUntil(p)
  } else {
    await p
  }
  return c.text('OK!')
})

But I think it should not have to look at the runtime key for that purpose. We should consider it.

@usualoma What do you think about this?

Anyway, if we change the behavior, it will be a breaking change that will be included in the next major release.

flexchar commented 2 months ago

As a related question, I am considering porting my worker to bun runtime in docker. I make use of c.executionCtx.waitUntil(p) many times.

Since Bun server would be long lived process, I would like at my own risk provide a "fake" object where I would provide my own waitUntil promise swallower.

@yusukebe, could you be as kind to show how I could inject it using app.use or on new Hono() or anywhere when initializing a new Hono? It would be very helpful!

yusukebe commented 2 months ago

Hi @flexchar

You can pass a waitUntil() function to the app.fetch(). This works well:

import { Hono } from 'hono'

const app = new Hono()

const log = async (message: string) => {
  await new Promise((resolve) => setTimeout(resolve, 1000))
  console.log(message)
}

app.get('/', (c) => {
  c.executionCtx.waitUntil(log(`Access from ${c.req.path}`))
  return c.json(0)
})

export default {
  fetch: (req: Request) =>
    app.fetch(req, undefined, {
      waitUntil: (fn) => fn, // <===
      passThroughOnException: () => {},
    }),
}