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

[FR] A new era of error handling #1094

Open
frederikhors opened this issue Sep 29, 2022 · 9 comments
Open

[FR] A new era of error handling #1094

frederikhors opened this issue Sep 29, 2022 · 9 comments
Labels
enhancement New feature or request

Comments

@frederikhors
Copy link
Contributor

frederikhors commented Sep 29, 2022

Ok, after a lot of reading/PRs/questions/answers I think I finally have a formal request to make.

Let's say I'm using in my app a custom AppError like the below:

pub struct AppError {
  pub public_message: Option<String>,
  pub operator_message: Option<String>,
  pub source: Option<&(dyn Error + 'static)>
}

I think we need:

  1. A way to convert this custom error to async_graphql::ServerError:

    1. A function? A callback? Or an impl From<AppError> for async_graphql::ServerError?

    2. I would like it to be "transparent" to the developer: I mean NO .map_err() in resolvers, only the ? usage;

  2. A function/callback to call when the resolver is done with the (original) error as argument to consume it (eg.: custom tracing, API call, DB insertion etc.):

    Example:

    pub async fn gql_handler(req: GraphQLRequest) -> GraphQLResponse {
      schema
        .execute(req.into_inner())
        .on_error(call_me_on_error_please)
        .await
        .into()
    }
    
    async fn call_me_on_error_please(err: AppError) {
      // do something with the original error here
    }

What do you think?

@frederikhors frederikhors added the enhancement New feature or request label Sep 29, 2022
@sunli829
Copy link
Collaborator

sunli829 commented Oct 1, 2022

You can do it like this:

pub async fn gql_handler(req: GraphQLRequest) -> GraphQLResponse {
   schema.execute(req.into_inner()).inspect_err(|err| {
     let Some(source) = err.source() { // get the error source
        if let Some(app_err) = source.downcast_ref::<AppError>() { // cast to AppError
          // do something
        }
      }
   }).await
}

@frederikhors
Copy link
Contributor Author

frederikhors commented Oct 1, 2022

I'm getting this error:

error[E0599]: no method named `inspect_err` found for opaque type `impl std::future::Future<Output = async_graphql::Response>` in the current scope
  --> src\graphql.rs:26:61
   |
26 |         .execute(req.into_inner()).inspect_err(|e| {
   |                                    ^^^^^^^^^^^ method not found in `impl std::future::Future<Output = async_graphql::Response>`

For more information about this error, try `rustc --explain E0599`.

@frederikhors
Copy link
Contributor Author

You can do it like this:

pub async fn gql_handler(req: GraphQLRequest) -> GraphQLResponse {
   schema.execute(req.into_inner()).inspect_err(|err| {
     let Some(source) = err.source() { // get the error source
        if let Some(app_err) = source.downcast_ref::<AppError>() { // cast to AppError
          // do something
        }
      }
   }).await
}

Why do you think this is not working?

@frederikhors
Copy link
Contributor Author

I've tried everything but can't get it to work.

Do I need to import any future::?

@phungleson
Copy link

phungleson commented Oct 9, 2022

I haven't tried it personally but it looks like you need to await. Something like this:

schema.execute(req.into_inner()).await.inspect_err

Okay, you probably need futures_util::TryFutureExt;

@frederikhors
Copy link
Contributor Author

Thanks, I tried adding:

use async_graphql::futures_util::TryFutureExt;

//..

pub async fn gql_handler(req: GraphQLRequest) -> GraphQLResponse {
   schema.execute(req.into_inner())
   .inspect_err() // error
   .await
}

the error is:

error[E0599]: the method `inspect_err` exists for opaque type `impl std::future::Future<Output = async_graphql::Response>`, but its trait bounds were not satisfied
   |
28 |         .inspect_err()
   |          ^^^^^^^^^^^ method cannot be called on `impl std::future::Future<Output = async_graphql::Response>` due to unsatisfied trait bounds
   |
   = note: the following trait bounds were not satisfied:
           `impl std::future::Future<Output = async_graphql::Response>: TryFuture`
           which is required by `impl std::future::Future<Output = async_graphql::Response>: TryFutureExt`
           `&impl std::future::Future<Output = async_graphql::Response>: TryFuture`
           which is required by `&impl std::future::Future<Output = async_graphql::Response>: TryFutureExt`
           `&mut impl std::future::Future<Output = async_graphql::Response>: TryFuture`
           which is required by `&mut impl std::future::Future<Output = async_graphql::Response>: TryFutureExt`

warning: unused import: `async_graphql::futures_util::TryFutureExt`
  |
2 | use async_graphql::futures_util::TryFutureExt;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

For more information about this error, try `rustc --explain E0599`.

@phungleson
Copy link

Yeah right, trait bound error, I should stop guessing. 🥲

@frederikhors
Copy link
Contributor Author

I think something should be done on this project to enable it. But I don't know what... :(

@Venryx
Copy link
Contributor

Venryx commented Mar 9, 2024

This compiled for me:

pub async fn handle_graphql_request(req: Request<AxumBody>, schema: RootSchema) -> Result<String, Error> {
	use async_graphql::futures_util::TryFutureExt;
	
	[...]
	
	let gql_response = schema.execute(gql_req).await.into_result();
	match gql_response {
		Ok(_) => {},
		Err(ref errors) => {
			for err in errors {
				error!("Test1:{:?}", err);
				if let Some(ref source) = err.source { // get the error source
					error!("Test2:{:?}", source);
					if let Some(ref app_err) = source.downcast_ref::<anyhow::Error>() { // cast to anyhow::Error
						error!("Test3:{:?}", app_err);
					}
				}
			}
		},
	}
	let response_str: String = serde_json::to_string(&gql_response)?;
	
	Ok(response_str)
}

So that seems to be an alternative way of implementing the "inspect_err" usage in @sunli829's post here: #1094 (comment)

However, when I run the code above, only the Test1 log message is seen; for some reason, the err.source field is None in my case. (naturally, making me unable to log the stack-trace information that should be present on the anyhow Error)

EDIT: The cause of this seems to be that the anyhow::Error is getting auto-converted to an async_graphql::ServerError, based on its implementation of Display (which does not include the error's backtrace), as seen here: #1265

EDIT2: Actually, disregard the two paragraphs above; I think this may just be due to some modifications I was making to the errors at another stage (of logging and stripping any backtraces from errors in my CustomExtension.request function). [also, I had forgotten to enable the "features" flag for the anyhow crate -- didn't realize it was necessary since most other error types it had been wrapping were already adding backtraces]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants