fix(structured-logger)!: correct types for hooks and other libraries

[BarryThePenguin]

The author should do the following, if applicable

• Add tests
• Run tests
• pnpm changeset at the top of this repo and push the changeset
• Follow the contribution guide

Using this middleware, I noticed a handful of rough edges..

• BaseLogger doesn't allow libraries that use a different log signature
• Hooks aren't typed; onRequest, onResponse, onError do not pass Context<Env> so types go missing
• The default hooks don't provide much value so are nearly always overridden

After playing around with a few different approaches, I landed on the following breaking changes

• Removing the BaseLogger interface, it was only needed because the default hooks used it. Without the default hooks, we no longer need the interface, allowing any kind of logging library to be used.
• The default hooks used properties such as { method, path, status, elapsedMs, err, }, but I think most consumers would provide their own hooks

With those two changes, the middleware now has the correct types, with the one caveat that consumers now have to provide their own hooks.

The one feature I did add is a skip method to not log certain requests. I included all the changes in one PR so others can see the whole thing.

Also @gabry-ts and @jansepke, if you're interested in having a look

[changeset-bot]

🦋 Changeset detected

Latest commit: 19089c5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@hono/structured-logger	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[gabry-ts]

Thanks for digging into this, and for the ping.

The hook typing problem is real and I should have caught it. Passing Context<Env> through to the hooks is clearly right. Same with dropping BaseLogger, honestly. I only wrote that interface so the defaults had something to call, and it ended up locking out any library whose signature doesn't look like pino's. Winston being the obvious casualty.

StructuredLoggerEnv is a better answer than what's in the README today. I'd just reverted an example that implied you could get c.var.logger typed from the middleware alone, which of course you can't, so an exported Env type to declare on the app fixes the thing I was papering over.

Where I'm hesitant is dropping the default hooks. Install the middleware, configure nothing, and you now get a logger on the context and no output at all. No error, no warning, just silence. You're right that most people override the defaults, but the defaults are also what makes this useful in the first five minutes, and "silent unless configured" is a rough default for a package named structured-logger. The README opener still promises automatic response time measurement too, which would need a rewrite.

Could we keep them and let people pass onResponse: false to opt out? Same escape hatch, without silence being what you get for free.

skip I like, and calling it before createLogger is the right order.

Curious what @jansepke thinks, and it's @yusukebe's call on the major bump either way.

[BarryThePenguin]

Yeah, agreed. I didn't want to drop the defaults either. Maybe the better change would be to make them required?

If we wanted to keep the defaults, we'd need to reintroduce some BaseLogger equivalent to make them work, which was one of the reasons for making the change in the first place. We can't have one without the other.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.22%. Comparing base (45c94d6) to head (19089c5).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2070      +/-   ##
==========================================
- Coverage   92.23%   92.22%   -0.01%     
==========================================
  Files         116      116              
  Lines        4122     4118       -4     
  Branches     1077     1076       -1     
==========================================
- Hits         3802     3798       -4     
  Misses        285      285              
  Partials       35       35              

Flag	Coverage Δ
structured-logger	100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.