Give accessors to wasmtime::Trap causes

[bnjbvr]

It wasn't possible to just retrieve a user-created error description in wasmtime::Trap (i.e. created with wasmtime::Trap::new("hi there")). This user message would only show up as part of the full Display impl for Trap, which also prints out the unmangled backtrace and other information. A first commit adds a user_message() accessor to retrieve it, if it was set.

As I was trying to find another way to get a custom error message, I saw that Trap implemented std::error::Error if the trap was created from an StdError. Unfortunately, it doesn't give access to it directly either, but it gives access to the underlying source() of the error itself. It would be nice to have access to the error itself, to not lose one layer of error information. As a result, I've tweaked source() so it returns the error that was used to create the trap, and not the error's source directly. Conceptually it makes sense that the first source of the trap is the error that it was converted from, but it would also be reasonable to say that the trap is the error (so instead, we could just add another accessor to get the error). Discuss. :)

[github-actions]

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

• peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[alexcrichton]

Could you describe a bit more the use case for getting the user-generated error message? Is the main goal to print the error without printing the backtrace?

Otherwise for implementing source the main thing to account for is that we want to make sure that error messages aren't double-printed. We print out a full chain of errors and if an error is returned from source then it shouldn't be printed by Display for Trap because that would otherwise mean the trap error message would get printed twice.

[bnjbvr]

Could you describe a bit more the use case for getting the user-generated error message? Is the main goal to print the error without printing the backtrace?

Yes, this is exactly my goal. Actually in our embedding the backtrace is post-processed for display, mainly for function names demangling, but now I'm seeing that Wasmtime does that for us. Our display tries to be minimal and display a bit less information, though. So getting the user-generated error message allows us to customize the error message that we display.

it shouldn't be printed by Display for Trap

It seems that Display for Trap doesn't iterate on an error sequence, but only shows the TrapReason and the backtrace generated from the frames. Am I missing something? In any case, I'm happy to provide an alternative error() accessor that returns the plain error (and keep the source() implementation as it was before this PR), or just to remove this last commit (if we can agree on getting the user message accessor).

[alexcrichton]

One option to fix this is to simply remove the backtrace printing from Trap, but personally I feel like it's the right default (I could be convinced otherwise though).

Another possibility to do this would be to implement something like fn display(&self) -> impl fmt::Display which returns basically what Display does minus the backtrace.

For printing twice, it's not actually Display for Trap that prints the error twice. A Display implementation for an error should print the message for just that error, and higher level libraries which generate reports, such as anyhow::Error, will do the iteration. I think that if you print out anyhow::Error where there's a trap inside (printed using {:?} which is anyhow's way of printing the full error backtrace) then you'll see the error printed twice.

I do think it's fine to implement source , though, it just involves not printing the error itself when we're of that variant. Instead a placeholder like "user-generated error" would be printed instead.

[bnjbvr]

We've discussed this a bit, and turns out that displaying the error reason is sufficient for my use case, while remaining abstract enough. Thanks Alex for the suggestions!

[bnjbvr]

After some thinking and discussion, another reasonable path would be to invert the default:

• have Display only print the oneliner reason,
• and have display_with_backtrace return an impl Display that showed the reason and the backtrace

This would be a (small) breaking change, and I'm not sure how people are using it, so I'm tempted to just keep the same default as it is right now, but could be convinced otherwise.

[alexcrichton]

Indeed I agree that's a possibility! (sorry meant to explain that before but it was brief)

I think the best way to figure this out thought might be to follow the Rust community norms in this regard (since Rust error libraries can often carry backtraces as well). Looking at anyhow it looks like explicit opt-in is required to enable backtraces (via env vars). This means that backtraces wouldn't be captured by default, but you could opt-in on a per-execution basis to doing so.

Would something like that be reasonable for y'all? It would be nice to somewhere have a note: use WASMTIME_BACKTRACE=1 to get a backtrace, but I'm not sure if impl Display for Trap is the best location to put that. I'm not sure where it would go...

Even if we go that route though I feel like we may still want this method so, even when backtraces are enabled, there's a specific way to print just the reason without the backtrace.

[bnjbvr]

This would work fine for our use case; curious to see other people's thoughts!

[alexcrichton]

Would you be up for sketching out what it would look like? Ideally we would actually avoid capturing a backtrace entirely on traps unless WASMTIME_BACKTRACE=1 I think?

[alexcrichton]

Hm actually on second thought after talking with some other folks I'm convinced that printing and capturing the backtrace by default is the right thing to do here. I think one thing we could do, though, is configure at a Store level whether backtraces are captured or not, but otherwise I think we'd want something like this method to print the reason of the trap.

Would you be ok with merging this and we can figure out other details later as-needed?

[bnjbvr]

Yes, sounds good to me!