honojs / node-server

Node.js Server for Hono
https://hono.dev
348 stars 43 forks source link

Support Node.js HTTP events for more granular control of server behavior. #106

Open rafaell-lycan opened 9 months ago

rafaell-lycan commented 9 months ago

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.

yusukebe commented 9 months ago

Hi @rafaell-lycan,

To handle Node.js specific matters, I'm considering passing IncomingMessage as part of the Bindings in @hono/node-server. This way, you can access it with c.env.IncomingMessage:

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 IncomingMessage, I think you can create a logger similar to morgan.

rafaell-lycan commented 9 months ago

@yusukebe I'm not sure if this would work on this scenario.

Consider it as a middleware that acts after the route resolution.

(request) -> [middleware] -> (route match) -> [handler] -> (event)

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)
rafaell-lycan commented 7 months ago

@yusukebe any ideas if this will be supported or any workaround to make it work?

yusukebe commented 7 months ago

@rafaell-lycan

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 finish. So, I might not yet fully understand it.

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 IncomingMessage from c.env and use that.

rafaell-lycan commented 6 months ago

@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'))
yusukebe commented 6 months ago

Hi @rafaell-lycan

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