Revisit panicking in std::fmt::Display implementations as a footgun

[Veetaha]

Panicking in std::fmt::Display implementations is a glaring footgun. Here is an example, where you are guaranteed to spend several hours debugging it, and potentially you will never find a cause because the panic happens in production deeply inside the logging system, which is your eyes, but they are now blind. Also, you won't notice this during development, because this log statement occurs in some obscure error-handling code which is hit 0.00001% of the time:

use std::collections::HashMap;
use tracing_subscriber::prelude::*;
use itertools::Itertools;

fn main() {
    let (loki, _join_handle) = tracing_loki::layer(
        "http://loki:3100".parse().unwrap(),
        HashMap::new(),
        HashMap::new(),
    )
    .unwrap();

    tracing_subscriber::registry()
        .with(tracing_subscriber::fmt::layer())
        // Commenting out the following line fixes the panic, but it's not obvious.
        // Basically any other layer that runs the `fmt::Display` implementation
        // of the `Itertools::format` second time after `fmt` subscriber may suffer from this panic
        .with(loki)
        .init();

    std::panic::set_hook(Box::new(move |_| {
        tracing::error!("panic happened");
    }));

    // panics here deep inside tracing when several layers display the value
    tracing::info!(field = %[1, 2, 3].iter().format(","), "Log event");

    eprintln!("This is never hit, and the panic `error!()` is never printed");
}

With this you'll see only this log output without any indication of panic:

Solution

Don't panic in std::fmt::Display implementations. Instead, the best approach would be to format a message with a lot of CAPS letters that say what the problem there is in the rendered message. Or panicking behavior could be made conditional upon a feature flag or maybe some other way of configuration, like a global variable or setup function