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

Please add sufficent error handling example with HTTP status codes to documentation #5549

Open
majda107 opened this issue Nov 17, 2022 · 6 comments
Labels
📚 documentation This issue is about working on our documentation. 🌶️ hot chocolate
Milestone

Comments

@majda107
Copy link

majda107 commented Nov 17, 2022

Is your feature request related to a problem?

In the current state of docs, it's unclear how the HTTP status interceptor determines the status code related to exception/returned data value in the resolver.

If you want to implement the 6a pattern and return 200 OK every time a "known" error occurs (for example validation issue), you have to do one of these things:
a) always return nullable! (otherwise, you'll get 500)
b) try-catch the exception -> report exception with resolverCtx.ReportError(...) -> return data/null.

Some people (including myself) don't automatically implement nullable return values, which results in non-sense 500 errors. This behavior of the HotChocolate pipeline should be documented in docs.

Note
This also affects errors related to authentication, if - for example - I return user data that are C# record (non-nullable by nature) and the user isn't authorized, it shoots out 500 HTTP code. When you make the user data record nullable, you get 200 HTTP code. (correct behavior, but this 'authorize' attribute should be namely documented because normally you don't even have to return null in resolver).

The solution you'd like

Add HotChocolate HTTP status code and error behavior in docs.

Product

Hot Chocolate

@majda107 majda107 changed the title [Documentation] Please add sufficent error handling with HTTP status codes documentation [Documentation] Please add sufficent error handling with HTTP status codes to documentation Nov 17, 2022
@majda107 majda107 changed the title [Documentation] Please add sufficent error handling with HTTP status codes to documentation [Documentation] Please add sufficent error handling example with HTTP status codes to documentation Nov 17, 2022
@michaelstaib
Copy link
Member

This actually is a wrong assumption.

When we talk about known error, or lets say domain errors. Things that are expected with stage 6a then you do not report it as an error et all.

resolverCtx.ReportError(...) is for real GraphQL errors.

Stage 6a is about exposing errors in you schema which we would do by either annotating the exceptions that are domain errors or using the MutationResult struct.

Essentially ...

[Error<SomeInputHasWrongFormatException>]
public async Task<SomeResult> DoSomething(string someInput)

@michaelstaib michaelstaib added 📚 documentation This issue is about working on our documentation. 🌶️ hot chocolate and removed 🎉 enhancement labels Nov 19, 2022
@glen-84
Copy link
Collaborator

glen-84 commented Jan 11, 2023

@michaelstaib

Does this mean that both FairyBread and AppAny.HotChocolate.FluentValidation are incorrectly calling ReportError?

If that's true:

  1. It might make sense to rename this method and/or clarify its expected use in the description. f.e. ReportGraphQlError, ReportFatalError, ReportRuntimeError, or whatever makes sense to separate it from application-level errors.
  2. How should these libraries handle errors? How do you report an application-level error without having to throw an exception and have every mutation configured to expect that exception? Is there something like ReportError for application errors?

As mentioned on Slack, a validation error when using the AppAny library is currently resulting in a 500 code, and an errors-only response (even when submitting multiple mutations in the same request, where one or more are successful). This is not good at all.

I really am looking forward to having comprehensive error handling documentation for v13, as well as part two of the video, covering error handling for queries.

Edit: There are other cases where we'd like to surface domain exceptions without having to annotate every single mutation. Maybe the mutation convention configuration could allow you to specify a list of exceptions to be included "globally"?

@John0x
Copy link
Contributor

John0x commented Aug 23, 2023

I'm currently running into this wall as well. I, personally, would prefer not to use the 6a convention (because of style preference).
I tried something like:

throw new GraphQLException(new ErrorBuilder().SetCode("AccountAlreadyExists").SetMessage("Account already exists.").Build());

But that doesn't seem to go along well with the apollo client (doesn't expose the errors information to the mutate function). Probably because of the 500 status code?

@glen-84 glen-84 changed the title [Documentation] Please add sufficent error handling example with HTTP status codes to documentation Please add sufficent error handling example with HTTP status codes to documentation Jan 23, 2024
@nesherhh
Copy link

What about Domain Errors on queries? [Error] seem to work only for mutations.

@glen-84
Copy link
Collaborator

glen-84 commented Feb 16, 2024

@nesherhh This is available in v14 previews. (#6846)

@hero3616
Copy link

Postman and BCP behaves differently in terms of response http status code with FairyBread. Postman returns 200 whereas BCP returns 500 if there is any input validator fails. The reason is that Postman in GraphQL mode always sends "Accept: application/json" header in the request, which tells HC server to use legacy mode (explained here https://chillicream.com/docs/hotchocolate/v13/server/http-transport). If you add the same header in BCP it doesn't return 500 but returns 200 just like Postman. If you don't want to add the header to client request you can configure the server.

public class CustomValidationErrorsHandler : DefaultValidationErrorsHandler
{
    public const string FairyBreadErrorCode = "VALIDATION_ERROR";

    protected override IErrorBuilder CreateErrorBuilder(
        IMiddlewareContext context,
        string argumentName,
        IValidator validator,
        ValidationFailure failure)
    {
        return base.CreateErrorBuilder(context, argumentName, validator, failure)
            .SetCode(FairyBreadErrorCode); // default is "FairyBread_ValidationError"
    }
}
public class CustomHttpResponseFormatter : DefaultHttpResponseFormatter
{
    public CustomHttpResponseFormatter(HttpResponseFormatterOptions options) : base(options)
    {
    }

    protected override HttpStatusCode OnDetermineStatusCode(IQueryResult result, FormatInfo format, HttpStatusCode? proposedStatusCode)
    {
        if (result.Errors?.Count > 0 &&
            result.Errors.Any(error => error.Code == CustomValidationErrorsHandler.FairyBreadErrorCode))
        {
            return HttpStatusCode.Conflict;
        }

        return base.OnDetermineStatusCode(result, format, proposedStatusCode);
    }
}
services.AddHttpResponseFormatter(_ => new CustomHttpResponseFormatter(new HttpResponseFormatterOptions
{
    HttpTransportVersion = HttpTransportVersion.Legacy,
    Json = new JsonResultFormatterOptions
    {
        Indented = true
    }
}));

@michaelstaib michaelstaib added this to the HC-14.0.0 milestone Jul 25, 2024
@michaelstaib michaelstaib modified the milestones: HC-14.0.0, HC-14.x.x Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 documentation This issue is about working on our documentation. 🌶️ hot chocolate
Projects
None yet
Development

No branches or pull requests

6 participants