tracing icon indicating copy to clipboard operation
tracing copied to clipboard

instrument ret try trait

Open Stargateur opened this issue 3 years ago • 3 comments

Feature Request

Crates

tracing

Motivation

I use Try trait that allow to use other type than Result or Option to use ? operator. The instrument have argument ret that allow to print return value, it's also have err cause ret have special behavior when value is Result printing only when Ok.

That mean I can't have similar behavior for my custom Try type.

Proposal

Remove err and so Result exception. Having a Ret trait that unify ret and err argument of instrument, by default ret would use Ret impl, but ret(Display) would use the trait Display and ret(Debug) would use the trait Debug.

trait Ret {
  fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result;
}

impl<T: Debug, E> Ret for Result<T, E> {
  fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
    match self {
      Ok(t) => write!(f, "{t:?}"),
      Err(_) => Ok(()),
  }
}

Alternatives

Doing alternative printing for everything that implement Try like Result However, this can only work well for the Continue branch. For break branch the type is self wrapped, this would mean for Ok(T) we show T but for Err(E) we show Result<!, E>. Also, this require unstable Try.

Stargateur avatar Jul 18 '22 00:07 Stargateur

Typically speaking, tracing and tracing-attributes only support types that are defined and stable within std. If we were to do this, I would prefer to have Try behind a Tokio-style unstable feature that can change in minor releases, but I worry about the diverging behavior inside of the tracing and tracing-attributes crate. I haven't thought through the implications of that approach, but I'm currently leaning to waiting for the Try trait to be stabilized. We're extremely cautious about adding new traits to the tracing crate and this would be the first unstable trait supported within tracing.

However, if this is blocking you, I'd suggest creating your own fork of tracing and tracing-attributes that uses the unstable Try trait. So long you depend on the same version of tracing-core, your forked crates will continue to just work with the broader tracing ecosystem.

davidbarsky avatar Jul 18 '22 15:07 davidbarsky

but I'm currently leaning to waiting for the Try trait to be stabilized.

My proposition is finally not related to Try, that more an idea to make ret and err more general. This could be used for any type. However I agree I didn't think a lot about this, for example, could be nice to have a way to check for terminal color behavior that user want. But my problem is real.

However, if this is blocking you, I'd suggest creating your own fork of tracing and tracing-attributes that uses the unstable Try trait. So long you depend on the same version of tracing-core, your forked crates will continue to just work with the broader tracing ecosystem.

Wow, thx, for now I just "accept" that my Display impl to something a little special. Good enough for a unreleased project.

Stargateur avatar Jul 19 '22 11:07 Stargateur

Sorry, to be clear: this is a very reasonable request and behavior to want. However, the combination of adding a new trait to tracing (generally, a difficult proposition due to its de-facto stability requirements and the high design requirements for including a new, non-sealed trait in tracing) and the fact that something like this will be in std sooner or later, I'm strongly inclined to wait for Try.

What I'm trying to say is: the people working on the Try trait have a better idea of the design for this and, by policy, we try to only integrate with types/values/trait that are in std inside of tracing because of risk of churn in tracing. Again: I think this is a good feature, but we don't have the bandwidth to design or add this to tracing for the above reasons.

davidbarsky avatar Jul 19 '22 15:07 davidbarsky