graphql-platform icon indicating copy to clipboard operation
graphql-platform copied to clipboard

Strawberry Shake No Longer Returning Client Errors in a Request Under Certain Circumstances

Open bryanh-bloomerang opened this issue 2 years ago • 3 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Product

Strawberry Shake

Describe the bug

I'm using Strawberrry Shake as a client in a suite of tests. Our tests cover many scenarios including those that should cause a validation error. Specifically, we have tests that send requests that have either empty or null parameters when they are specifically not allowed. Those tests then verify that the proper validation error comes back.

If it has any bearing on this issue, our GraphQL gateway that I am testing against is running HotChocolate.

We had been using v13.5.1 for a while and today I tried to upgrade to v13.7.0. This causes our tests covering the scenarios I described above to start failing. From what I can tell, the full validation error is no longer being properly deserialized by Strawberry Shake

Here is an example of a request sent with an empty parameter value (that should not be):

{
  "id": "e921924e5acc17d99e15700782fe315e",
  "query": "mutation CreateDatabaseConnection($database: CreateDatabaseConnectionInput!) { createDatabaseConnection(input: $database) { __typename databaseConnection { __typename id name } errors { __typename ... on Error { code statusCode message } ... on DatabaseConnectionAlreadyExistsError { databaseName service } ... on InvalidDatabaseConnectionFormatError { databaseName service } ... on UserManagementServiceError { service } ... on Auth0UnavailableError { service } } } }",
  "operationName": "CreateDatabaseConnection",
  "variables": {
    "database": {
      "databaseName": ""
    }
  }
}

And here is the response I get back. As you can see, the status code is 500 and the response contains no data object. This is expected.

HTTP/1.1 500 Internal Server Error
Date: Fri, 01 Dec 2023 21:15:10 GMT
Content-Type: application/json; charset=utf-8
Transfer-Encoding: chunked
Connection: keep-alive
Server: Kestrel

{
  "errors": [
    {
      "message": "NonEmptyStringType cannot parse the provided literal. The provided string was empty.",
      "path": [
        "database",
        "databaseName"
      ],
      "extensions": {
        "code": "HC0002",
        "actualType": "NonEmptyString",
        "field": "CreateDatabaseConnectionInput.databaseName",
        "fieldType": "NonEmptyString"
      }
    }
  ]
}

While the raw response I get back is correct regardless if I use v13.5.1 or v13.7.0, the resulting response object is not.

I debugged my tests in my IDE and below is the result object returned by Strawberry Shake.

This screenshot is using v13.5.1. Screenshot 2023-12-01 at 4 27 50 PM

And this screenshot is using v13.7.0 Screenshot 2023-12-01 at 4 29 22 PM

As you can see, I get the full error in the first ClientError in v13.5.1. In v13.7.0, all I am seeing is a generic message stating "Response status code does not indicate success: 500 (Internal Server Error). None of the other properties on the ClientError object are populated.

After some trial and error, I discovered that this issue was introduced in v13.6.0-preview.33. Up until that version, everything was working as expected.

I should also note that I have seen something similar before in this issue https://github.com/ChilliCream/graphql-platform/issues/5684#issuecomment-1463836382

Steps to reproduce

Since our GraphQL gateways are not open for public use, this is the best way I can describe to duplicate the issue.

  1. Find a GraphQL gateway that has a mutation that takes in multiple parameters. At least one of the parameters must not be allowed to be empty. The gateway should return a validation error and a 500 response if the parameter is empty.
  2. Submit a request to the gateway using Strawberry Shake with that mutation and make sure the parameter is an empty string.
  3. Look at the OperationResult that is returned. The ClientError array should have a single object. The details of the validation error should be present but are not.

Relevant log output

No response

Additional Context?

No response

Version

13.7.0

bryanh-bloomerang avatar Dec 01 '23 21:12 bryanh-bloomerang

Any update on this one? It makes it difficult to write and troubleshoot unit tests.

greggbjensen avatar May 03 '24 21:05 greggbjensen

This is kind of the correct behavior.

Content-Type: application/json; charset=utf-8

If content-type is application/json then a non 200 status code is a total failure. If content-type is application/graphql-response+json than we no its a graphql response and should pick up the error.

We switched to the spec compliant transport layer somewhere in the 13 cycle.

https://github.com/graphql/graphql-over-http

michaelstaib avatar May 03 '24 22:05 michaelstaib

What server version are you using of HotChocolate?

michaelstaib avatar May 03 '24 23:05 michaelstaib