fix(log): guard non-Level "level" attribute against panic - #559
fix(log): guard non-Level "level" attribute against panic#559garmr-ulfr wants to merge 3 commits into
Conversation
The slog handler asserted a.Value.Any().(slog.Level) unchecked in the LevelKey case, panicking the process on any log call passing a non-Level value keyed "level". Guard with a comma-ok assertion; on mismatch emit a group (level=WARN, an error attr, and the original value under a typed key) instead of crashing.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe slog attribute replacement logic now safely handles unexpected level value types, preserving valid level formatting and returning diagnostic attributes instead of panicking. ChangesLog level formatting fix
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Hardens the log package’s slog handler by making ReplaceAttr resilient to callers that log a "level" attribute whose value is not a slog.Level, preventing a process panic and preserving additional context in the emitted log entry.
Changes:
- Adds a comma-ok type assertion for
"level"attributes before callingFormatLogLevel. - On type mismatch, emits a grouped set of attributes describing the mismatch instead of panicking.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Hardens the radiance slog handler so a log call passing a non-
slog.Levelvalue keyed"level"can no longer panic the process.Changes
log/log.go, theReplaceAttrslog.LevelKeycase asserteda.Value.Any().(slog.Level)without a comma-ok check. Replaced with a checked assertion; on a type mismatch the handler now emits a group (level=WARN, anerrorattr describing the unexpected type, and the original value preserved under a typedlevel-<T>key) instead of crashing.Bug Fixes
"level"with a non-slog.Levelvalue crashed the whole process (previously hit by memmon's memory-tick log passing"level"as a string, worked around in302a3caby renaming the key). The handler is now panic-safe for that key regardless of the caller's value type.Summary by CodeRabbit