spin icon indicating copy to clipboard operation
spin copied to clipboard

Opaque errors at times returned from the SDKs

Open rylev opened this issue 2 years ago • 6 comments

We're usually directly forwarding errors that have been created by wit-bindgen from SDK APIs. This can lead to errors that are less than ideal and only offer the minimal amount of information to the user.

For example, calling key_value::Store::open when the spin.toml manifest does not contain a key_value_stores key for the component simply displays the following:

Handler returned an error: Error::AccessDenied

If the user is unfamiliar with the key_value feature, they have absolutely no idea that this error means that they do not have any allowed stores configured. Ideally we would show something like the following:

Handler returned an error: Error::AccessDenied

Note: Are you sure you've included a list of allowed key_value_stores in your spin.toml? 
Make sure to add the `key_value_stores` key to your component and specify which key value stores you'd like to allow. 

For example, `key_value_stores = ["default"]` allows the default key value store.  

Unfortunately, each SDK would have to handle this themselves. The Rust SDK could do this by making the Display implementation more sophisticated than it currently is. However, to make it really nice where we have configurable "verbosity" of error reporting, we'd likely want a bespoke trait that extends std::error::Error and provides additional features.

rylev avatar Apr 06 '23 08:04 rylev

Great idea @rylev - in the meantime I have added a callout box to the docs so that folks can find a resolution to this outside of the CLI's response. https://github.com/fermyon/developer/pull/1278

tpmccallum avatar May 09 '24 00:05 tpmccallum

Unfortunately, each SDK would have to handle this themselves

Is that really the case? Could we do this handling on the runtime side?

radu-matei avatar May 09 '24 06:05 radu-matei

Could we do this handling on the runtime side?

We could log details, but we can't tell whether an error is exceptional or not so we'd end up logging sometimes when the error is already being properly handled by the guest.

lann avatar May 09 '24 13:05 lann

As we make other breaking changes to interfaces we should also adopt something like wasi:io/error to help with this.

lann avatar May 09 '24 13:05 lann

but we can't tell whether an error is exceptional or not so we'd end up logging sometimes when the error is already being properly handled by the guest.

I think the main question is if you'd rather have scenarios with no error details at all or sometimes have duplicated errors.

radu-matei avatar May 09 '24 13:05 radu-matei

We could certainly add it at a non-default log level but I personally prefer to keep default logging as actionable as possible.

lann avatar May 09 '24 13:05 lann