Skip to content

fix: some error handling and improvements#25

Open
gabyx wants to merge 2 commits into
mainfrom
feat/review-and-some-error-handling
Open

fix: some error handling and improvements#25
gabyx wants to merge 2 commits into
mainfrom
feat/review-and-some-error-handling

Conversation

@gabyx
Copy link
Copy Markdown
Contributor

@gabyx gabyx commented Oct 3, 2024

  • Some error handling and some obsolete stuff.

@gabyx gabyx force-pushed the feat/review-and-some-error-handling branch from 95f816c to e2e6181 Compare October 3, 2024 20:32
Comment thread fuzon-http/src/main.rs
// Shared app state built from config and used by services
#[derive(Clone, Debug)]
struct AppState {
// TODO: Add doc what these dict maps.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// TODO: Add doc what these dict maps.
// collection name -> associated term matcher

Comment thread fuzon-http/src/main.rs
struct Config {
host: String,
port: u16,
// TODO: Add doc what these dict maps.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// TODO: Add doc what these dict maps.
// collection name -> list of ontology paths

Comment thread fuzon-http/src/main.rs
Comment on lines +140 to +149
// TODO: This is unsafe: not sure what to do though -> maybe block the executor and set env
// variables before this asyn function is started.
// Strange that Rust compiled anyway, without the unsafe block, any idea?
unsafe {
env::set_var(
"RUST_LOG",
"fuzon_http=info,actix_web=warn,actix_server=info",
);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

question: how is this unsafe? If I remove the unsafe directive, everything works fine. This function is in a sync context and we only start the runtime after exporting, right?

Comment thread fuzon-http/src/errors.rs
@@ -0,0 +1,7 @@
#[derive(Debug, thiserror::Error, actix_web_error::Json)]
#[status(BAD_REQUEST)] // default status for all variants
pub enum ApiError {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

praise: very cool approach 💯
question: iiuc, as we expand the API, we would add variants for the different failure cases to this enum?

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.

2 participants