Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

default error formatting in fn main() -> Result<(),Error> using Debug instead of Display #39

Open
ghost opened this issue May 12, 2021 · 8 comments

Comments

@ghost
Copy link

ghost commented May 12, 2021

The default error formatting when you use the ? operator in main() leaves much to be desired. From the output, it appears to use Debug, which is often automatically derived and will often print very unfriendly walls of characters, especially in combination with the new experimental backtrace api:

Error: ParseError { kind: InsufficientBytes { required: 4, received: 3 }, backtrace: Backtrace [{ fn: "err::parse", file: "./main.rs", line: 35 }, { fn: "err::main", file: "./main.rs", line: 57 }, { fn: "core::ops::function::FnOnce::call_once", file: "/rustc/9d9c2c92b834c430f102ea96f65119e37320776e/library/core/src/ops/function.rs", line: 227 }, { fn: "std::sys_common::backtrace::__rust_begin_short_backtrace", file: "/rustc/9d9c2c92b834c430f102ea96f65119e37320776e/library/std/src/sys_common/backtrace.rs", line: 125 }, { fn: "std::rt::lang_start::{{closure}}", file: "/rustc/9d9c2c92b834c430f102ea96f65119e37320776e/library/std/src/rt.rs", line: 66 }, { fn: "core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once", file: "/rustc/9d9c2c92b834c430f102ea96f65119e37320776e/library/core/src/ops/function.rs", line: 259 }, { fn: "std::panicking::try::do_call", file: "/rustc/9d9c2c92b834c430f102ea96f65119e37320776e/library/std/src/panicking.rs", line: 379 }, { fn: "std::panicking::try", file: "/rustc/9d9c2c92b834c430f102ea96f65119e37320776e/library/std/src/panicking.rs", line: 343 }, { fn: "std::panic::catch_unwind", file: "/rustc/9d9c2c92b834c430f102ea96f65119e37320776e/library/std/src/panic.rs", line: 431 }, { fn: "std::rt::lang_start_internal", file: "/rustc/9d9c2c92b834c430f102ea96f65119e37320776e/library/std/src/rt.rs", line: 51 }, { fn: "std::rt::lang_start", file: "/rustc/9d9c2c92b834c430f102ea96f65119e37320776e/library/std/src/rt.rs", line: 65 }, { fn: "main" }, { fn: "__libc_start_main" }, { fn: "_start" }] }

whereas the Display impl looks pretty good, and very similar to panic![] and the formatting I would expect from other languages or gdb:

required 4 bytes while parsing but received 3
   0: err::parse
             at ./main.rs:35:18
   1: err::main
             at ./main.rs:46:19
   2: core::ops::function::FnOnce::call_once
             at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/core/src/ops/function.rs:227:5
   3: std::sys_common::backtrace::__rust_begin_short_backtrace
             at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/sys_common/backtrace.rs:125:18
   4: std::rt::lang_start::{{closure}}
             at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/rt.rs:49:18
   5: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once
             at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/core/src/ops/function.rs:259:13
      std::panicking::try::do_call
             at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/panicking.rs:379:40
      std::panicking::try
             at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/panicking.rs:343:19
      std::panic::catch_unwind
             at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/panic.rs:431:14
      std::rt::lang_start_internal
             at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/rt.rs:34:21
   6: std::rt::lang_start
             at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/rt.rs:48:5
   7: main
   8: __libc_start_main
   9: _start

It seems like Display is almost always what you want to see in this fn main() context (with some very light wrapping to feature-detect for backtraces) if you haven't set up a custom error reporter of your own. After reading through every issue in this repo and many error-related issues in the rust repo, I still have some questions:

  1. why does main() use Debug to print errors instead of Display?
  2. is it realistic/possible to change this behavior or are we pretty much stuck with it?

I think it would be really unfortunate if one of the first things you needed to do when picking up rust is to learn about error reporters so that you can debug your program effectively. Somehow failure prints errors in the second format, perhaps by formatting Display-like output in its Debug implementation. Is that also a viable option if we're stuck with Debug in fn main() forever? It's very hard to find information about what is going on with rust errors right now.

I have been writing some libraries and I was removing failure since it has been deprecated, but now I am completely at a loss to figure out what I'm supposed to do to provide a nice end-user error experience, as the current core rust behavior seems like a regression over what failure provides.

Here is the program I wrote to reproduce this behavior:

#![feature(backtrace)]
type Error = Box<dyn std::error::Error+Send+Sync>;
use std::backtrace::{BacktraceStatus,Backtrace};

#[derive(Debug)]
pub struct ParseError {
  kind: ParseErrorKind,
  backtrace: Backtrace,
}

#[derive(Debug)]
pub enum ParseErrorKind {
  InsufficientBytes { required: usize, received: usize }
}

impl std::error::Error for ParseError {
  fn backtrace<'a>(&'a self) -> Option<&'a Backtrace> {
    Some(&self.backtrace)
  }
}

impl std::fmt::Display for ParseError {
  fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
    match &self.kind {
      ParseErrorKind::InsufficientBytes { required, received } => {
        write!(f, "required {} bytes while parsing but received {}", required, received)
      }
    }
  }
}

fn parse(src: &[u8]) -> Result<u32,ParseError> {
  if src.len() < 4 {
    Err(ParseError {
      backtrace: Backtrace::capture(),
      kind: ParseErrorKind::InsufficientBytes { required: 4, received: src.len() }
    })
  } else {
    Ok(u32::from_be_bytes([src[0],src[1],src[2],src[3]]))
  }
}

fn main() -> Result<(),Error> {
  let buf = vec![1,2,3,4,5,6,7,8];
  println!["# desired main() error formatting (via Display + backtrace):\n"];
  if let Err(e) = parse(&buf[0..3]) {
    match std::error::Error::backtrace(&e) {
      Some(bt) => match bt.status() {
        BacktraceStatus::Captured => eprint!["{}\n{}", e, bt],
        _ => eprint!["{}", e],
      },
      None => eprint!["{}", e],
    }
  }
  println!["\n"];
  println!["# default main() error formatting (via Debug):\n"];
  parse(&buf[0..3])?;
  Ok(())
}
@yaahc
Copy link
Member

yaahc commented May 12, 2021

Hey @substack, thank you for opening this issue. We've discussed this issue in the context of stabilizing the Termination trait in the past. We cannot change the bound on the Termination impl for Result from Debug to Display because this API has already been stabilized, which would make this a breaking change.

That said, we do already have a plan for fixing this. We're going to specialize the Termination impl for E: Error, which we can do because Error is a subtrait of Debug, so it is strictly more specific. Where as Display is an independent trait with no relationship to Debug in the type system, which would make it ambiguous if we were to try to specialize on Debug vs Display.

The only reason we haven't done this already is because there is a soundness issue when specializing trait implementations that could be conditional based on lifetimes. This will require niko's proposed work around for always applicable trait impls being implemented. Once we have this in place though we hope to update the Termination impl on Result to correctly leverage the Error trait when possible so that you don't get the garbled and lossy output that is often presented via Debug.

@BurntSushi
Copy link
Member

BurntSushi commented May 12, 2021

1. why does main() use Debug to print errors instead of Display?

This is where at least some discussion of that happened that led to deciding on Debug: rust-lang/rust#43301 (comment)

I just re-read it, and from what I can tell, I think the thinking was that ?-in-main was going to be for quick throw-away stuff or examples, and that "production" CLI tools would want to do their own thing anyway.

is it realistic/possible to change this behavior or are we pretty much stuck with it?

I had sadly not realized when this was getting stabilized back then, otherwise I like to think I would have spoken up. Almost right after it stabilized, I asked about reversing course, but it's quite difficult. IIRC, @yaahc has a plan for this, but I can't recall where I read those plans. I can't find them in this repo.

Currently, the anyhow crate is personally what I use in CLI tools. It prints the "display" version of an error via its Debug impl. If you compile with Rust nightly, you get backtraces. There is also eyre and snafu. Thankfully, the ecosystem doesn't need to pick one of these. They should all be largely inter-compatible.

@programmerjake
Copy link
Member

Hmm, could this be changed to work better over an edition change? I would think so since main is not generally public API of whatever crate is writing fn main

@yaahc
Copy link
Member

yaahc commented May 13, 2021

Hmm, could this be changed to work better over an edition change? I would think so since main is not generally public API of whatever crate is writing fn main

Possibly. It might be possible to change how the edition handles the return value of main to use a different version of the Termination trait, we cannot do so backwards compatibly via the current Termination trait, since std is shared between editions.

@programmerjake
Copy link
Member

Hmm, could this be changed to work better over an edition change? I would think so since main is not generally public API of whatever crate is writing fn main

Possibly. It might be possible to change how the edition handles the return value of main to use a different version of the Termination trait, we cannot do so backwards compatibly via the current Termination trait, since std is shared between editions.

Switching to a new Termination trait sounds like a good idea! We'd probably want a annotation of some sort on fn main to allow using the old Termination trait from a new edition to allow automatic edition upgrades to retain behavior. Maybe:

#[termination(edition=2018)]
fn main() -> Result<(), impl Debug> {
    todo!()
}

I think writing an RFC of some sort soon would be good so that we could possibly get it into edition 2021. @yaahc would you be interested in doing that?

@yaahc
Copy link
Member

yaahc commented May 13, 2021

I think writing an RFC of some sort soon would be good so that we could possibly get it into edition 2021. @yaahc would you be interested in doing that?

I don't think I can commit to writing another RFC right now, I feel like I'm already at the limit of how much work I can handle and writing has been particularly difficult for me recently. Also, I feel like it's too close to the 2021 edition to be able to really bake a solution to this problem, so I'd prefer to take it slow and aim for the 2024 edition instead, assuming we even want to make this change. I'm hoping that the specialization approach will be sufficient and obviate the need for this proposal.

@programmerjake
Copy link
Member

I don't think I can commit to writing another RFC right now, I feel like I'm already at the limit of how much work I can handle and writing has been particularly difficult for me recently.

Hope it all goes well for you!

Also, I feel like it's too close to the 2021 edition to be able to really bake a solution to this problem, so I'd prefer to take it slow and aim for the 2024 edition instead, assuming we even want to make this change. I'm hoping that the specialization approach will be sufficient and obviate the need for this proposal.

Ok, sounds good enough to me!

@kshelekhin
Copy link

It would also be nice to be able to replace Error with something. For example, it's a common pattern to print program name:

$ ./foobar
foobar: cannot remove "/": Permission denied

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants