Stream: coqbot devs & users

Topic: addComment failures


view this post on Zulip 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?

view this post on Zulip Théo Zimmermann (Apr 21 2021 at 08:24):

What is MutationWithErrorResult?

view this post on Zulip 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

view this post on Zulip 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.

view this post on Zulip 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

view this post on Zulip 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?)

view this post on Zulip 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.

view this post on Zulip Jason Gross (Apr 21 2021 at 13:58):

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

view this post on Zulip 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.

view this post on Zulip 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?

view this post on Zulip 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: Jan 31 2023 at 09:01 UTC