Repository: brettcannon/gidgethub (python)

Issue:
GraphQL response content-type header assertion failing
Love your library design BTW.

Small issue I thought I would call out. I'm using Github enterprise server 2.19.7 and the `content-type` header on the response to the `/api/graphql` request is different than the rest api. By one space to be exact.

There's an assertion on line 242 that expects the content type to equal `application/json; charset=utf-8`, and it does in response a v3 rest api call, but the v4 graphql api returns `application/json;charset=utf-8`.

https://github.com/brettcannon/gidgethub/blob/48db737541537d0df78dac2cfac717e86d158e7a/gidgethub/abc.py#L242

It's easy enough to work around (because your library is awesome), I'm just doing this for now:

```python
class MyGitHubAPI(GitHubAPI):
    async def _request(
        self, method: str, url: str, headers: Mapping[str, str], body: bytes = b""
    ) -> Tuple[int, Mapping[str, str], bytes]:
        # Add proxy auth token
        headers = dict(**headers, **generate_proxy_header())
        # return await super(MyGitHubAPI, self)._request(method, url, headers, body)
        resp_status, resp_headers, resp_data = await super(MyGitHubAPI, self)._request(
            method, url, headers, body
        )
        resp_headers = dict(resp_headers)
        resp_headers["content-type"] = "application/json; charset=utf-8"
        return resp_status, resp_headers, resp_data
```

Pull request:
Fix abc.GitHubAPI.graphql() to accept response content types lacking spaces
## Background

Closes #121 

As stated in the issue, Github enterprise server sets a response header equal to `application/json;charset=utf-8` (note: without the space separating the type and character encoding). 

The graphql method in the ABC asserts the content type header equals `application/json; charset=utf-8` (note: with space). 

The failing assertion prevents the method from successfully returning.

## Change
* Removed the content type assertion from the graphql response handling
* Replaced the response data decoding with a call to `sansio` to parse the content-type from the header and load the json based on that.
  * It felt kind of dirty copying this logic out of `sansio._decode_body()` but it did have a little bit of v3 specific logic in there. Maybe a `_decode_gql_body()` is needed?
* Added a new explicit exception when the graphql response content type is unknown to gidgethub, `GraphQLResponseTypeError`.

## Test Plan
Added three tests to prevent regressions and test the new exception. I appended the original issue number just in case anyone is curious about the context of the tests. If that's not your style, I can remove it.
* `test_response_content_type_parsing_gh121`
* `test_unknown_response_content_type_gh121`
* `test_no_response_content_type_gh121`


Patch:
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 8e6155d..224f318 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -37,7 +37,6 @@ jobs:
           file: ./cov.xml
 
   lint:
-    if: github.event.action != 'closed' || github.event.pull_request.merged
     runs-on: ubuntu-latest
 
     steps:
diff --git a/docs/__init__.rst b/docs/__init__.rst
index 1229533..b2bf3a3 100644
--- a/docs/__init__.rst
+++ b/docs/__init__.rst
@@ -138,3 +138,17 @@ GraphQL-specific
    An exception representing an error relating to the GraphQL query itself.
 
    Inherits from :exc:`GraphQLException`.
+
+.. exception:: GraphQLResponseTypeError(content_type, response)
+
+   An exception raised when a GraphQL query response is not JSON.
+
+   Inherits from :exc:`GraphQLException`.
+
+   .. attribute:: content_type
+
+      The value of the content-type header in the response from GitHub.
+
+   .. attribute:: response
+
+      The decoded response value from GitHub.
diff --git a/docs/sansio.rst b/docs/sansio.rst
index afc1c0e..aa7ef63 100644
--- a/docs/sansio.rst
+++ b/docs/sansio.rst
@@ -44,7 +44,7 @@ without requiring the use of the :class:`Event` class.
 
    .. attribute:: data
 
-      The `payload <https://developer.github.com/webhooks/#payloads>`_ of the
+      The `payload <https://developer.github.com/webhooks/event-payloads/>`_ of the
       event.
 
 
diff --git a/gidgethub/__init__.py b/gidgethub/__init__.py
index 46ababd..5ce118b 100644
--- a/gidgethub/__init__.py
+++ b/gidgethub/__init__.py
@@ -2,7 +2,7 @@
 __version__ = "4.1.0"
 
 import http
-from typing import Any
+from typing import Any, Optional
 
 
 class GitHubException(Exception):
@@ -133,3 +133,13 @@ class QueryError(GraphQLException):
 
     def __init__(self, response: Any) -> None:
         super().__init__(response["errors"][0]["message"], response)
+
+
+class GraphQLResponseTypeError(GraphQLException):
+
+    """The GraphQL response has an unexpected content type."""
+
+    def __init__(self, content_type: Optional[str], response: Any) -> None:
+        super().__init__(
+            f"Response had an unexpected content-type: '{content_type!r}'", response
+        )
diff --git a/gidgethub/abc.py b/gidgethub/abc.py
index 122a8a9..2ab1650 100644
--- a/gidgethub/abc.py
+++ b/gidgethub/abc.py
@@ -11,6 +11,7 @@ from . import (
     GraphQLAuthorizationFailure,
     GraphQLException,
     QueryError,
+    GraphQLResponseTypeError,
 )
 from . import sansio
 
@@ -239,8 +240,19 @@ class GitHubAPI(abc.ABC):
         status_code, response_headers, response_data = await self._request(
             "POST", endpoint, request_headers, request_data
         )
-        assert response_headers["content-type"] == _json_content_type
-        response = json.loads(response_data.decode("utf-8"))
+
+        if not response_data:
+            raise GraphQLException("Response contained no data", response_data)
+
+        # Decode content.
+        resp_content_type = response_headers.get("content-type")
+        type_, encoding = sansio._parse_content_type(resp_content_type)
+        response_str = response_data.decode(encoding)
+        if type_ == "application/json":
+            response: Dict[str, Any] = json.loads(response_str)
+        else:
+            raise GraphQLResponseTypeError(resp_content_type, response_str)
+
         if status_code >= 500:
             raise GitHubBroken(http.HTTPStatus(status_code))
         elif status_code == 401:
diff --git a/gidgethub/httpx.py b/gidgethub/httpx.py
index 4c7a4b0..d8b9b6a 100644
--- a/gidgethub/httpx.py
+++ b/gidgethub/httpx.py
@@ -16,7 +16,13 @@ class GitHubAPI(gh_abc.GitHubAPI):
     ) -> Tuple[int, Mapping[str, str], bytes]:
         """Make an HTTP request."""
         response = await self._client.request(
-            method, url, headers=dict(headers), data=body
+            method,
+            url,
+            # Reason: see discussion at https://github.com/brettcannon/gidgethub/pull/122#issuecomment-633738024.
+            # httpx 0.13.1 updated the HeaderTypes and it is acting invariant, causing our Mapping[str, str] to be
+            # technically incompatible with their Mapping[Union[str, bytes], Union[str, bytes]].
+            headers=dict(headers),  # type: ignore
+            data=body,
         )
         return response.status_code, response.headers, response.content
 


Test patch:
diff --git a/tests/test_abc.py b/tests/test_abc.py
index 4547b72..21a597c 100644
--- a/tests/test_abc.py
+++ b/tests/test_abc.py
@@ -15,6 +15,7 @@ from gidgethub import (
     GraphQLException,
     QueryError,
     RedirectionException,
+    GraphQLResponseTypeError,
 )
 from gidgethub import abc as gh_abc
 from gidgethub import sansio
@@ -804,3 +805,43 @@ class TestGraphQL:
         with pytest.raises(GraphQLException) as exc:
             await gh.graphql("does not matter")
         assert exc.value.response == response_data
+
+    @pytest.mark.asyncio
+    async def test_response_content_type_parsing_gh121(self):
+        gh, response_data = self.gh_and_response("success-200.json")
+        # Test that a JSON content type still works if formatted without spaces.
+        gh.response_headers["content-type"] = "application/json;charset=utf-8"
+        # Should not fail.
+        resp = await gh.graphql("does not matter")
+        assert resp is not None
+        # Spaces in the content-type header should not be a problem.
+        gh.response_headers["content-type"] = "application/json; charset=utf-8"
+        # Should not fail.
+        resp = await gh.graphql("does not matter")
+        assert resp is not None
+
+    @pytest.mark.asyncio
+    async def test_unknown_response_content_type_gh121(self):
+        gh, response_data = self.gh_and_response("success-200.json")
+        # A non-JSON response should raise an exception.
+        gh.response_headers["content-type"] = "application/gidget;charset=utf-8"
+        with pytest.raises(GraphQLResponseTypeError):
+            await gh.graphql("does not matter")
+
+    @pytest.mark.asyncio
+    async def test_no_response_content_type_gh121(self):
+        gh, response_data = self.gh_and_response("success-200.json")
+        # An empty content type should raise an exception.
+        gh.response_headers["content-type"] = ""
+        with pytest.raises(GraphQLException):
+            await gh.graphql("does not matter")
+        del gh.response_headers["content-type"]
+        with pytest.raises(GraphQLException):
+            await gh.graphql("does not matter")
+
+    @pytest.mark.asyncio
+    async def test_no_response_data(self):
+        # An empty response should raise an exception.
+        gh = MockGitHubAPI(200, body=b"", oauth_token="oauth-token",)
+        with pytest.raises(GraphQLException):
+            await gh.graphql("does not matter")
diff --git a/tests/test_tornado.py b/tests/test_tornado.py
index c80d7a3..958ccf6 100644
--- a/tests/test_tornado.py
+++ b/tests/test_tornado.py
@@ -18,7 +18,7 @@ class TornadoTestCase(AsyncTestCase):
         gh = gh_tornado.GitHubAPI("gidgethub")
         await gh.sleep(delay)
         stop = datetime.datetime.now()
-        assert (stop - start) > datetime.timedelta(seconds=delay)
+        assert (stop - start) >= datetime.timedelta(seconds=delay)
 
     @tornado.testing.gen_test
     async def test__request(self):