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

feat: use custom error types in internal and external crates #182

Merged
merged 4 commits into from
Aug 24, 2023

Conversation

ereslibre
Copy link
Contributor

@ereslibre ereslibre commented Jul 19, 2023

Builds on top of #181, although that PR is not strictly necessary for this one.

Fixes: #73

@ereslibre
Copy link
Contributor Author

Marking as ready for review!

@ereslibre ereslibre requested a review from a team August 23, 2023 14:45
@ereslibre ereslibre force-pushed the error-types branch 2 times, most recently from 8da9884 to bb4e3e1 Compare August 23, 2023 15:41
Copy link
Contributor

@Angelmmiguel Angelmmiguel left a comment

Choose a reason for hiding this comment

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

The PR is really great! It simplifies the way you can programmatically manage errors on Wasm Workers Server. I added a few comments, but most of them are just consolidating the names between different crates:

CouldNot -> Cannot
ErrorXXX -> XXXError

I'm gonna approve it in advance, but please take a look at the suggestions before merging it :)

crates/config/src/lib.rs Show resolved Hide resolved
crates/runtimes/src/errors.rs Outdated Show resolved Hide resolved
crates/runtimes/src/lib.rs Outdated Show resolved Hide resolved
crates/runtimes/src/lib.rs Outdated Show resolved Hide resolved
crates/runtimes/src/modules/external.rs Outdated Show resolved Hide resolved
crates/worker/src/errors.rs Outdated Show resolved Hide resolved
crates/worker/src/lib.rs Outdated Show resolved Hide resolved
crates/worker/src/lib.rs Outdated Show resolved Hide resolved
src/utils/errors.rs Outdated Show resolved Hide resolved
src/utils/errors.rs Outdated Show resolved Hide resolved
@Angelmmiguel Angelmmiguel added this to the v1.5.0 milestone Aug 24, 2023
@Angelmmiguel Angelmmiguel added the 🚀 enhancement New feature or request label Aug 24, 2023
Copy link
Contributor

@Angelmmiguel Angelmmiguel left a comment

Choose a reason for hiding this comment

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

LGTM! 👏

@ereslibre ereslibre merged commit 8b8c333 into vmware-labs:main Aug 24, 2023
5 checks passed
@ereslibre ereslibre deleted the error-types branch August 24, 2023 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve overall error capturing
3 participants