Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose Wasm errors in stderr #131

Closed
Angelmmiguel opened this issue May 22, 2023 · 5 comments
Closed

Expose Wasm errors in stderr #131

Angelmmiguel opened this issue May 22, 2023 · 5 comments
Assignees
Labels
🚀 enhancement New feature or request 👋 good first issue Good for newcomers

Comments

@Angelmmiguel
Copy link
Contributor

Angelmmiguel commented May 22, 2023

Is your feature request related to a problem? Please describe.

When processing a request in a worker, failures may happen at two different levels:

  • Recoverable errors in the worker module: the module recovers from the error. It may print relevant information to stderr and continue processing the request or providing an error response.
  • Wasm failures: the Wasm module fails and the worker cannot recover. This may be cause by a panic in the worker, unreachable functions, or other issues. The module cannot provide a proper response and error message. wws returns the error (Wasm backtrace) to the handler.

In the first case, we're already printing any data sent to stderr from the worker. After 5207684, we're inheriting the stderr from the environment and configuring it in the WASI context. You can also configure the stderr when using wws as a library.

However, for the second case we're not printing any error and just providing a default error page:

let (handler_result, handler_success) =
match route
.worker
.run(&req, &body_str, store, vars, stderr_file.get_ref())
{
Ok(output) => (output, true),
Err(_) => (WasmOutput::failed(), false),
};

This error exposes the Wasm backtrace. This information is very relevant for debugging on both development and production.

Describe the solution you'd like

Before sending a generic error, wws must print the Wasm backtrace in the terminal. The stderr is configurable, so the stderr_file (Option<File>) may contain a file descriptor that should be use. If it's None, you can directly call eprintln! with the proper message.

There's a different task to improve how wws manages errors (#73)

Describe alternatives you've considered

None

Additional context

@Angelmmiguel Angelmmiguel added the 🚀 enhancement New feature or request label May 22, 2023
@Angelmmiguel Angelmmiguel added the 👋 good first issue Good for newcomers label May 22, 2023
@carrycooldude
Copy link
Contributor

Will love to work on this issue @Angelmmiguel

@Angelmmiguel
Copy link
Contributor Author

That would be amazing @carrycooldude! I’m gonna assign the task to you just for tracking 😄.

Feel free to drop any question you may have about the issue.

@carrycooldude
Copy link
Contributor

Just need to ask @Angelmmiguel , I just have to add Error Msg right?

@Angelmmiguel
Copy link
Contributor Author

Just need to ask @Angelmmiguel , I just have to add Error Msg right?

Exactly @carrycooldude. The goal is to capture the error from the run method and display it in the stderr. However, the stderr is configurable, so it may be a specific file to write or the default stderr from the process. For that, the handler defines an Option<File> variable called stderr_file.

If that variable is present, you should write the data to that file descriptor. If it's not present, you can just print the error using eprintln!.

I forgot to add that to the issue, so I also updated the description accordingly.

@Angelmmiguel
Copy link
Contributor Author

Closed by #141 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 enhancement New feature or request 👋 good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants