Handling Serialization Errors

[mrvaruntandon]

Pull request checklist

• Please read our contributor guide
• Consider creating a discussion on the discussion forum
  first
• Make sure the PR doesn't introduce backward compatibility issues
• Make sure to have sufficient test cases

Pull Request type

• Bugfix
• Feature
• Refactoring (no functional changes, no api changes)
• Build related changes
• Other (please describe):

Changes in this PR

• Introduced a ErrorDetail Type for Serialization Errors
• Added the new ErrorDetail Type while TypedGraphQLError is being formed in GraphQLJavaErrorInstrumentation

Describe the new behavior from this PR, and why it's needed
This PR resolves #1914

Alternatives considered

Describe alternative implementation you have considered

[Meemaw]

I would argue coercing errors (which the original issue is about) are client errors (not internal server errors).

[mrvaruntandon]

Hi @Meemaw,

I would argue coercing errors (which the original issue is about) are client errors (not internal server errors).

I checked the Coercing Interface, there are three Coercing*Exception, one for each method in the interface. I could only come up with scenarios where parseLiteral() and serialize() in the interface would be invoked to throw their respective exceptions and this PR is based on that understanding.

Coercing.parseLiteral() invoked to resolve an input from a query variable into a Java object, if such parsing fails CoercingParseLiteralException is thrown. I tried to replicate this scenario and found my implementation would throw a client error like below:

{
  "errors": [
    {
      "message": "Validation error (WrongType@[ip]) : argument 'domain' with value 'StringValue{value='google'}' is not a valid 'Domain' - Invalid domain name",
      "locations": [
        {
          "line": 11,
          "column": 6
        }
      ],
      "path": [
        "ip"
      ],
      "extensions": {
        "classification": "ValidationError",
        "errorType": "BAD_REQUEST",
        "errorDetail": "INVALID_ARGUMENT"
      }
    }
  ]
}

This is not an INTERNAL errorType, but a client error type BAD_REQUEST.

The INTERNAL errorType would be returned in cases where the server fails to successfully invoke the serialize() and throws CoercingSerializeException.

Please let me know you are referring to which scenario about coercing errors being client errors and I could test for it and handle it appropriately.

[mrvaruntandon]

IMO in summary, CoercingParseLiteralException should be treated as client errors and CoercingSerializeException should be treated as server errors.

[mrvaruntandon]

@paulbakker @Emily @kilink @srinivasankavitha could one of you please review this PR?

[srinivasankavitha]

Thanks for the fix! PR changes look good to me.

[mrvaruntandon]

Hi @srinivasankavitha, could you please tell me why the build associated with this PR is failing? Do I need to take any action to fix the build?

[kilink]

@mrvaruntandon it looks like it's failing due to ktlint errors, you can run ./gradlew formatKotlin to fix them.

[mrvaruntandon]

@kilink, thank you for your guidance!

I have fixed the lint issue, could you please trigger the build again and merge this PR if successful?

[mrvaruntandon]

Thank you @srinivasankavitha @kilink @Meemaw for your support. This was my first open source contribution.