Stream: coqbot devs & users

Topic: addComment failures

Jason Gross (Apr 20 2021 at 20:53):

@Théo Zimmermann I'm trying to understand how to get access to error information. According to https://docs.github.com/en/graphql/overview/explorer,

mutation addComment {
  payload: addComment(input: {subjectId: "foo", body: "foo"}) {
    commentEdge {
      node {
        url
      }
    }
  }
}

gives a response of

{
  "data": {
    "payload": null
  },
  "errors": [
    {
      "type": "NOT_FOUND",
      "path": [
        "payload"
      ],
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "message": "Could not resolve to a node with the global id of 'foo'"
    }
  ]
}

This suggests that we want to be using MutationWithErrorResult rather than Mutation somewhere, I think? But addComment is listed in bot-components/schema.json as living under Mutation.

@Théo Zimmermann do you understand what's going on here?

Théo Zimmermann (Apr 21 2021 at 08:24):

What is MutationWithErrorResult?

Théo Zimmermann (Apr 21 2021 at 08:27):

In any case, what surprises me is that the HTTP code is a success, otherwise we would be getting an Error and the full body of the response would be displayed in the log, according to https://github.com/coq/bot/blob/e0f545cdc74d0c12bd30c82b47bc9e5ca3b7cafd/bot-components/GraphQL_query.ml#L21-L29

Théo Zimmermann (Apr 21 2021 at 08:28):

Anyway, now I'm thinking that whatever the HTTP code is, we should check for the "errors" field and return it is present.

Jason Gross (Apr 21 2021 at 11:58):

I got MutationWithErrorResult from https://github.com/reasonml-community/graphql-ppx/blob/855b16bca8621ae5f0a1179edc8bf46061248e8b/graphql_schema.json#L1301 and https://github.com/reasonml-community/graphql-ppx/blob/855b16bca8621ae5f0a1179edc8bf46061248e8b/schema.graphql#L99 and https://github.com/reasonml-community/graphql-ppx/blob/855b16bca8621ae5f0a1179edc8bf46061248e8b/tests_native/mutation.re#L4 . I wasn't able to decipher things enough to really grock it

Jason Gross (Apr 21 2021 at 12:01):

I think the "new way of returning errors" that we read about is this, where the API will return both errors and partial data, and it's left to the client to decide whether "I got half your data but not the other half, here are the reasons" is an error or not. So indeed I think we should check for the errors field and if it's a list with at least one error then we should return those errors (maybe along with the partial data?)

Théo Zimmermann (Apr 21 2021 at 12:12):

That sounds reasonable, though I don't know how to manage the case of result + error. I might need to switch from the result monad to something else.

Jason Gross (Apr 21 2021 at 13:58):

Instead of returning ('r, 'e) result you could return ('r, 'r option * 'e) result?

Théo Zimmermann (Apr 21 2021 at 14:02):

I was thinking we would need also to be able to report several errors if at some point we have a partial result and an error and get a new error by acting on the partial result.

Jason Gross (Apr 21 2021 at 14:17):

Already I believe errors is a list when we get it from GitHub, so maybe we should make 'e be a list?

Théo Zimmermann (Apr 21 2021 at 15:06):

Yes, that sounds reasonable. This would then give ('r, 'r option * string list) result.

Last updated: Apr 18 2024 at 16:01 UTC