Skip to content
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

Open
rafaell-lycan opened this issue Nov 30, 2023 · 6 comments

Comments

@rafaell-lycan
Copy link

rafaell-lycan commented Nov 30, 2023

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
Copy link
Member

yusukebe commented Dec 5, 2023

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
Copy link
Author

rafaell-lycan commented Dec 13, 2023

@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
Copy link
Author

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

@yusukebe
Copy link
Member

yusukebe commented Feb 8, 2024

@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
Copy link
Author

rafaell-lycan commented Feb 14, 2024

@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:

  • You add a middleware which will collect all the data you need;
  • On every request, the middleware sets defines a requestId and startAt used later to calculate how long the full request took to complete including the request status (SLA/SLO control), path, user-agent and a few other bits

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants