Don't panic on debug when packets aren't utf-8 #156
Conversation
The current debug logging assumes a packet is utf-8, and if not, it panics, which is bad. This provides a fallback output if the packet is not utf-8, and introduces a `debug` module to add tools to for debug builds and debugging in general. Closes #151
markmandel
force-pushed
the
bug/debug-utf8
branch
from
1b2f2b5 to
faf2e7b
Compare
src/debug.rs
Outdated
| @@ -0,0 +1,22 @@ | |||
| /* | |||
There was a problem hiding this comment.
Can we nest the file under a directory e.g src/utils/debug.rs, src/helpers/debug.rs etc? If we have lots of these helper modules the src/ directory structure could get confusing e.g it'll be hard to tell looking at the folder structure what src/debug.rs is
There was a problem hiding this comment.
Can we nest the file under a directory e.g src/utils/debug.rs
I like it! I'll make this change 👍
I'm thinking we should log these messages at trace rather than debug since this is probably as low level a message as it gets?
I keep going back and forth on this - I really like having these at debug level, and I think the information is very useful for debugging low-traffic problems, and having it readily available is handy. But I also hear that in production systems, this can become more signal than noise (which can be fixed with tooling, but that's a whole other thing).
Sounds like we need a tie breaker 😄 @luna-duclos do you want to weigh in here?
There was a problem hiding this comment.
I'd say detailed packet dumping should be trace, its rarely needed and you'd usually want it on a node thats not receiving significant traffic.
There was a problem hiding this comment.
From Discord:
Kebbe • Luna Duclos Today at 10:39 AM
imo, trace.
Well, it seems like the prevailing opinion is trace level - so I defer to the everyone else expert opinions! I'll make this change.
|
Updates made! PTAL! |
The current debug logging assumes a packet is utf-8, and if not, it panics, which is bad.
This provides a fallback output if the packet is not utf-8, and introduces a
debugmodule to add tools to for debug builds anddebugging in general.
Closes #151