Skip to content

fix: better error handling when plugin runs out of fuel #762

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 4, 2024
Merged

fix: better error handling when plugin runs out of fuel #762

merged 2 commits into from
Sep 4, 2024

Conversation

zshipko
Copy link
Contributor

@zshipko zshipko commented Sep 3, 2024 •
edited
Loading

  • Checks error results to determine if a plugin ran out of fuel, if the fuel is 0 after we get the error, then we return an out of fuel error message instead.
  • Updates extism_plugin_error to check Plugin::error_msg regardless of the kernel's error state

@zshipko zshipko requested review from chrisdickinson and evacchi and removed request for chrisdickinson September 3, 2024 22:12
@zshipko
Copy link
Contributor Author

zshipko commented Sep 3, 2024 •
edited
Loading

@evacchi - this should fix the segfault you were running into. it seems like the fuel was running out before the plugin could set an error which was causing it to get into a weird state.

if this works for you I will merge and make a new release.

evacchi
evacchi approved these changes Sep 4, 2024
View reviewed changes
Copy link
Contributor

@evacchi evacchi left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change effectively causes my plugin to consistently fail with plugin ran out of fuel for all values below a certain threshold, including down to 0 👍 let's also merge #760 so that we can depend on it in the java-sdk

@zshipko zshipko merged commit c2866a7 into main Sep 4, 2024
5 checks passed
@zshipko zshipko deleted the fix-libextism-fuel branch September 4, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evacchi evacchi evacchi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zshipko @evacchi