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

enhancement: remove panics and expects #1395

Merged
merged 2 commits into from
Oct 5, 2023
Merged

Conversation

lostman
Copy link
Contributor

@lostman lostman commented Oct 4, 2023

Description

Closes #1098.

This PR cleans up remaining expects, unwraps and panics in the executor and service.

The only place where expects are still present is the Drop implementation of WasmArg. However, since WasmArg is created (and destroyed) in a spawn_blocking task, it will not affect the indexer service. If Drop were to panic!, the individual indexer would shut down (and would not be restarted automatically). The indexer service would be unaffected.

Testing steps

CI test should pass.

Changelog

  • Added more error variants to WasmIndexerError. Changed expects and unwraps to returning Err variant which also triggers the early exit.
  • Instead of panic!-ing or printing error! and break-ing the loop, the executor now returns an IndexerResult. The Err variant is printed when it reaches IndexerService run loop.

@lostman lostman changed the title remove panics enhancement: remove panics and expects Oct 4, 2023
@lostman lostman marked this pull request as ready for review October 4, 2023 14:01
@lostman lostman self-assigned this Oct 4, 2023
@ra0x3 ra0x3 merged commit 8f7b76a into develop Oct 5, 2023
19 checks passed
@ra0x3 ra0x3 deleted the maciej/no-panic-or-expect branch October 5, 2023 20:12
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

Successfully merging this pull request may close these issues.

Remove all panic and expect from executor codepaths
3 participants