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

Let build-time plugin runners return Results #234

Closed
wingertge opened this issue Nov 22, 2022 · 3 comments
Closed

Let build-time plugin runners return Results #234

wingertge opened this issue Nov 22, 2022 · 3 comments

Comments

@wingertge
Copy link
Contributor

The only way to display errors from external build tools executed by plugins currently is to panic! in the plugin code. This is very unergonomic and adds a lot of noise to the output.

There should be a facility to return Results from server side actions and have them reported to the user via command line output, just like Rust build errors. As far as I can tell there's already a facility in the runner types for return types (which is currently just () for all build time actions). This could be changed to Result and the results could be handled by printing to the console.

An optional further improvement that might be possible to implement with a separate plugin error type would be fatal vs non-fatal errors so a plugin can either make the build fail or let it succeed with a warning. This is more of a nice to have feature though, I think failing the build should be the default since if a build tool fails something about the code is very wrong and will put the application in an unintended state, even if that state is just CSS not working as intended. Catching errors on compile instead of during debugging is one of the core philosophies of Rust after all.

@github-actions
Copy link

This repo is set up to triage issues with Tribble, but this issue couldn't be processed. If you deliberately didn't use Tribble to report this issue, you can safely ignore this warning. If you did, something's gone wrong here.

@arctic-hen7
Copy link
Member

This should make it into the next beta version!

@arctic-hen7
Copy link
Member

Done! All plugin actions must now return Result<T, E>, where T is the plugin action's return type, and E is Box<dyn std::error::Error> (best used with .into() on any error type). Engine-side plugin errors are handled through the usual mechanism, and displayed by the CLI with exit code 1, while browser-side plugin errors (which have to be supported, since the settings actions in particular are used on both sides) will result in a panic! handled by the user's given panic handler, if one has been provided.

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

No branches or pull requests

2 participants