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

Bad API behaviour results in uncaught errors #94

Open
varon opened this issue Jan 28, 2024 · 5 comments
Open

Bad API behaviour results in uncaught errors #94

varon opened this issue Jan 28, 2024 · 5 comments

Comments

@varon
Copy link

varon commented Jan 28, 2024

An incorrect query to the Monday.com GraphQL API results in the following error:

{
  "error_code": "ColumnValueException",
  "status_code": 200,
  "error_message": "This status label doesn\u0027t exist, possible statuses are: {0: Working on it, 1: Done, 2: Stuck, 3: Started}",
  "error_data": {
    "column_value": "{\"label\"\u003d\u003e\"Bottling\"}",
    "column_id": "status"
  }
}

This returns status code 200, and bypasses all error detection in the code for the success path, because it's looking for only the key "errors" - perhaps we can expand this via config? Any ideas on how to handle that?

Thanks for making such a great tool.

@varon
Copy link
Author

varon commented Jan 28, 2024

While I love the simplicity of this tool, it would be great to add in some kind of custom error checker - we might be able to do this by attaching a callback into the generated GraphQL client at runtime? That would at least serve as a perfect place to inject our code:

    member _.UpdateItemAsync(input: UpdateItem.InputVariables) =
        task {
            let query = """
                mutation UpdateItem($BoardId: ID!, $ItemId: ID!, $ColumnValues: JSON!) {
                    update_item: change_multiple_column_values(board_id: $BoardId, item_id: $ItemId, column_values: $ColumnValues) {
                        id,
                        name
                    }
                }
            """
            let inputJson = JsonConvert.SerializeObject({ query = query; variables = Some input }, settings)
            let! response = httpClient.PostAsync(url, new StringContent(inputJson, Encoding.UTF8, "application/json"))

            let! responseContent = Async.AwaitTask(response.Content.ReadAsStreamAsync())
            use sr = new StreamReader(responseContent)
            use tr = new JsonTextReader(sr)
            let responseJson = serializer.Deserialize<JObject>(tr)

            match response.IsSuccessStatusCode with
            | true ->
                // *************
                // HERE <--- call some optional callback to add validation/error checking based on the remote API's behavior.
                // *************
                let errorsReturned =
                    responseJson.ContainsKey "errors"
                    && responseJson.["errors"].Type = JTokenType.Array
                    && responseJson.["errors"].HasValues

                if errorsReturned then
                    let response = responseJson.ToObject<GraphqlErrorResponse>(serializer)
                    return Error response.errors
                else
                    let response = responseJson.ToObject<GraphqlSuccessResponse<UpdateItem.Query>>(serializer)
                    return Ok response.data

            | errorStatus ->
                let response = responseJson.ToObject<GraphqlErrorResponse>(serializer)
                return Error response.errors
        }

@Zaid-Ajaj
Copy link
Owner

Hi @varon it is very unfortunate that errors returned by a GraphQL API are not typed anywhere 😞 I've added one configuration point called customErrorType which would allow to override the shape of the errors. However the problem here is that the key "errors" is hardcoded. All the GraphQL services I worked with when developing this returned an errors array so I assumed that is the convention.

Although we could extend this via configuration so that "errors" is not hardcoded, it is not enough because the error returned by the API is an object rather an array so it is not as simple as changing "errors" to be a configurable key.

There is one possible extension point though, the generated clients accept a custom HttpClient that the user can provide. HTTP requests made by HttpClient .NET can be intercepted by implementing a custom delegation handler. I don't have a sample code but you could technically intercept and modify the response JSON so that the result changes from:

{
  "error_code": "ColumnValueException",
  "status_code": 200,
  "error_message": "This status label doesn\u0027t exist, possible statuses are: {0: Working on it, 1: Done, 2: Stuck, 3: Started}",
  "error_data": {
    "column_value": "{\"label\"\u003d\u003e\"Bottling\"}",
    "column_id": "status"
  }
}

into:

{
  "errors": [{ "error_code": "...", "status_code": 200, "error_message": "..." }]
}

Then using the custom error type you can tell the serializer to handle things like error_code, status_code etc.

@Zaid-Ajaj
Copy link
Owner

Another solution is changing code generation so that it accepts a simpler interception transform without having to customize the HttpClients but that requires doing so for .NET and Fable so it's a lot of work.

@varon
Copy link
Author

varon commented Jan 29, 2024

Thanks for the prompt and detailed responses, @Zaid-Ajaj.

Monday.com has a fairly horrendous API, it's far from anything that's decent. Their dates for instance are encoded as nested, escaped JSON strings with separate fields for dates and times (WTF!).

Will go the HttpClient route, but would be good to have that work via a more elegant mechanism in the future.

Would still love to have some kind of custom validator jsonResponse -> Error list function we can call in the success response case.

@Zaid-Ajaj
Copy link
Owner

Monday.com has a fairly horrendous API, it's far from anything that's decent. Their dates for instance are encoded as nested, escaped JSON strings with separate fields for dates and times (WTF!).

That sounds horrible 😞 sorry to hear that. It is probably an API that is auto-generated from from their data source as is.

Will go the HttpClient route, but would be good to have that work via a more elegant mechanism in the future.

If you do find a solution, can you post it here? Then I can add a section to the documentation

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

No branches or pull requests

2 participants