-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Node.js HTTP events for more granular control of server behavior. #106
Comments
Hi @rafaell-lycan, To handle Node.js specific matters, I'm considering passing import { Hono } from 'hono'
import { serve } from '@hono/node-server'
import { IncomingMessage } from 'http'
type Bindings = {
IncomingMessage: IncomingMessage
}
const app = new Hono<{ Bindings: Bindings }>()
app.get('/', (c) => {
return c.text(`You access from ${c.env.IncomingMessage.socket.remoteAddress}`)
})
serve(app) With access to |
@yusukebe I'm not sure if this would work on this scenario. Consider it as a middleware that acts after the route resolution.
How this implementation would look like? I was digging a bit on this pkg implementation but I haven't found where we could potentially create a mediator/event-dispatcher for it (probably on Hono's core). I believe this could be a simple feature that can be enabled/disabled at the config level. import { Hono, MiddlewareHandler } from 'hono'
import { serve } from '@hono/node-server'
// It could be always enabled when used with `@hono/node-server`
// and optional (different implementation) for Edge.
const app = new Hono({ enableEvents: true })
const eventLoggerMiddleware = (): MiddlewareHandler => async (ctx, next) => {
// Available with Context
ctx.on('finish', () => {
// Magic goes here.
})
await next()
}
// OR with custom events `onEvent` with Hono
app.onFinished((ctx) => {
// Magic goes here.
})
app.use('*', eventLoggerMiddleware())
app.get('/', (ctx) => {
return ctx.text('My shining new API')
})
serve(app) |
@yusukebe any ideas if this will be supported or any workaround to make it work? |
I'm not super familiar with Node.js. The mental model of Node.js is different from the Web Standard API used by Cloudflare Workers and others. These runtimes don't use the timing like a I think you already know it, but f it is only to be executed at the very end, you can write this: const app = new Hono()
app.use(async (c, next) => {
await next()
// logging
})
app.get('/', (c) => c.json('handler')) Or, with PR #129, you can now take the |
@yusukebe what I'm trying to accomplish is essentially migrate a few middleware (Express) which calculates the total time of the request and send the data to my metrics collector (OTL/Datadog) later. Implementation-wise you imagine the following scenario:
Would this be possible? app.use(async (c, next) => {
const startTime = performance.now()
const requestId = c.req.header('x-request-id') ?? uuid()
await next()
const tags = {
...getContextTags(), // additional tags from an AsyncLocalStorage
request_id: requestId,
path: c.req.routePath,
"user-agent": c.req.header('User-Agent'),
method: c.req.method,
// HTTP Code - No idea how to retrieve this | Express: res.statusCode.toString()
response_code: c.res.status // ?,
}
const duration = Math.floor(performance.now() - startTime)
collector.timing('http.request_latency', duration, tags)
})
app.get('/', (c) => c.json('handler')) |
I think you can do it. I think you can declare that middleware on top of the handlers/middleware. The following Logger middleware implementation would be helpful: https://github.com/honojs/hono/blob/main/src/middleware/logger/index.ts |
Hi, I've created a discussion but it's worth adding it here as well.
Essentially Node.js has its own HTTP events such as
close
|error
|finish
|timeout
that can be used with middleware like morgan to log/collect data and more.To add more context, we use it to collect metrics such as response time, request ID (UUID generated by another middleware), and other things.
The text was updated successfully, but these errors were encountered: