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

Centralize error handling #36

Open
n-dusan opened this issue May 14, 2024 · 0 comments
Open

Centralize error handling #36

n-dusan opened this issue May 14, 2024 · 0 comments
Assignees

Comments

@n-dusan
Copy link
Contributor

n-dusan commented May 14, 2024

Have all the exit process calls be stored in a single, centralized location.

This is a minor point, but generally we only want one central call to `exit()` in a program. 
The idea being that we bubble up all errors with `?` and only at some common overarching point 
in the code do we decide to exit based on the received error. 
This can get hard with threads and async, etc. But ideally, 
centralising the `exit()` can help us avoid some unexpected bugs and 
generally improive architecture. But don't worry about it too much, it's just something to bear in mind.

Originally posted by @tombh in #33 (comment)

@n-dusan n-dusan self-assigned this May 14, 2024
n-dusan added a commit that referenced this issue Jun 11, 2024
Progress on #36

Added `CliError` enum that maps any/all errors related to working with
stelae CLI.

Updated `run` to not return any errors, but instead to decide how to
stop the process. We expect to currently return error code 1 on any
found stelae errors, and instruct the users to inspect the local logs.

Still have issues with centralizing errors found during initializing
actix `App` instance. The reason this error handling is tricky is
because we're passing in the generic, opaque `App` type to both `app.rs`
and `routes.rs`. I tried initializing the `init_app` in `app.rs` outside
of the `HttpServer::new(..)`, but the problem was that `App` did not
implement `Clone` trait, so could not get it working quickly. This is
something we'd like to resolve in the future, so leave the process
exits uncentralized for now, and leave a comment in `app.rs`.
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

1 participant