Skip to content

SyncClient: update session with transport's headers #203

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.

Sign up for GitHub

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

Open
shtrom wants to merge 2 commits into
base: main
Choose a base branch
from
+45 −5
Open

SyncClient: update session with transport's headers #203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a97d868
test_client: test that REST requests have the correct headers
shtrom Aug 25, 2026
9f64417
SyncClient: update session with transport's headers
shtrom Aug 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/simple_github/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from requests import Response as RequestsResponse
from requests import Session
from requests.adapters import HTTPAdapter
from requests.structures import CaseInsensitiveDict
from urllib3.util.retry import Retry

if TYPE_CHECKING:
Expand Down Expand Up @@ -141,6 +142,13 @@ def _get_requests_session(self) -> Session:
session = self._get_gql_session()
assert isinstance(session.transport, RequestsHTTPTransport)
assert session.transport.session

# mozilla-releng/simple-github#202: work around graphql-python/gql#613.
if session.transport.headers:
session.transport.session.headers = CaseInsensitiveDict(
session.transport.headers
)

Comment on lines +146 to +151

@Eijebong Eijebong Aug 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This got 2 issues.

It's not in the right place. It should be in _get_gql_session or doing GQL -> REST -> GQL with the same object ends up with the second gql call behaving different.

And secondly, this overrides all headers instead of updating them. So we lose default headers (notably Accept-Encoding and User-Agent).

Something like this test shows both well:

def test_headers(responses, sync_client):
    defaults = set(requests.Session().headers)
    print("Default:", defaults)
    responses.post(GITHUB_GRAPHQL_ENDPOINT, status=200, json={"data": {"foo": "bar"}})
    responses.get(f"{GITHUB_API_ENDPOINT}/octocat", status=200, json={"answer": 42})

    sync_client.execute("query { foo }")
    before = dict(responses.calls[-1].request.headers)
    print("Before:", before)

    sync_client.get("/octocat")
    sync_client.execute("query { foo }")
    after = dict(responses.calls[-1].request.headers)
    print("After:", after)

    session = sync_client._get_requests_session()
    assert defaults <= set(session.headers)
    assert before == after
    assert session.headers["Accept"] == "application/vnd.github+json"
    assert session.headers["Authorization"] == f"Bearer {sync_client.auth._token}"

You get:

Default: {'Accept-Encoding', 'User-Agent', 'Accept', 'Connection'}
Before: {'User-Agent': 'python-requests/2.34.2', 'Accept-Encoding': 'gzip, deflate, br, zstd', 'Accept': 'application/vnd.github+json', 'Connection': 'keep-alive', 'Authorization': 'Bearer abc', 'Content-Length': '24', 'Content-Type': 'application/json'}
After: {'Accept': 'application/vnd.github+json', 'Authorization': 'Bearer abc', 'Content-Length': '24', 'Content-Type': 'application/json'}

I'm 95% sure the UA missing gets saved by urllib3 later on and that it'll work anyway (github requires a UA) but it's a very weird behavior and I don't want to have to debug something 6 months from now because the UA changes depending on request order

return session.transport.session

def _get_retry_session(self) -> Session:
Expand Down
42 changes: 37 additions & 5 deletions test/test_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import pytest
import pytest_asyncio
import requests
from aiohttp import ClientResponseError
from gql import Client as GqlClient
from gql.client import ReconnectingAsyncClientSession, SyncClientSession
Expand Down Expand Up @@ -168,21 +169,25 @@ async def test_async_client_rest(aioresponses, async_client):
resp = await client.get("/octocat")
result = await resp.json()
assert result == {"answer": 42}
assert_correct_aiorequest_headers(aioresponses, url, "GET")

aioresponses.post(url, status=200, payload={"answer": 42})
resp = await client.post("/octocat", data={"foo": "bar"})
result = await resp.json()
assert result == {"answer": 42}
assert_correct_aiorequest_headers(aioresponses, url, "POST")

aioresponses.put(url, status=200, payload={"answer": 42})
resp = await client.put("/octocat", data={"foo": "bar"})
result = await resp.json()
assert result == {"answer": 42}
assert_correct_aiorequest_headers(aioresponses, url, "PUT")

aioresponses.patch(url, status=200, payload={"answer": 42})
resp = await client.patch("/octocat", data={"foo": "bar"})
result = await resp.json()
assert result == {"answer": 42}
assert_correct_aiorequest_headers(aioresponses, url, "PATCH")

aioresponses.delete(url, status=200)
await client.delete("/octocat")
Expand All @@ -198,13 +203,26 @@ async def test_async_client_rest(aioresponses, async_client):
# internal aiohttp-retry tracking data
trace_request_ctx=mock.ANY,
)
assert_correct_aiorequest_headers(aioresponses, url, "DELETE")

aioresponses.get(url, status=401)
with pytest.raises(ClientResponseError):
resp = await client.get("/octocat")
resp.raise_for_status()


def assert_correct_aiorequest_headers(aioresponses, url: str, method: str = "GET"):
aioresponses.assert_called_with(
url,
method=method,
args_to_match=["headers"],
headers={
"Accept": "application/vnd.github+json",
"Authorization": "Bearer abc",
},
)


@pytest.mark.asyncio
async def test_async_client_retries_on_5xx(aioresponses, async_client):
client = async_client
Expand All @@ -222,39 +240,53 @@ def test_sync_client_rest(responses, sync_client):
client = sync_client
url = f"{GITHUB_API_ENDPOINT}/octocat"

responses.get(url, status=200, json={"answer": 42})
get_mock = responses.get(url, status=200, json={"answer": 42})
resp = client.get("/octocat")
result = resp.json()
assert result == {"answer": 42}
assert_correct_request_headers(get_mock.calls[0].request)

responses.post(url, status=200, json={"answer": 42})
post_mock = responses.post(url, status=200, json={"answer": 42})
resp = client.post("/octocat", data={"foo": "bar"})
result = resp.json()
assert result == {"answer": 42}
assert_correct_request_headers(post_mock.calls[0].request)

responses.put(url, status=200, json={"answer": 42})
put_mock = responses.put(url, status=200, json={"answer": 42})
resp = client.put("/octocat", data={"foo": "bar"})
result = resp.json()
assert result == {"answer": 42}
assert_correct_request_headers(put_mock.calls[0].request)

responses.patch(url, status=200, json={"answer": 42})
patch_mock = responses.patch(url, status=200, json={"answer": 42})
resp = client.patch("/octocat", data={"foo": "bar"})
result = resp.json()
assert result == {"answer": 42}
assert_correct_request_headers(patch_mock.calls[0].request)

responses.delete(url, status=200)
delete_mock = responses.delete(url, status=200)
client.delete("/octocat")
resp = responses.calls[-1].response
assert resp.url == url
assert resp.request.method == "DELETE"
assert resp.status_code == 200
assert_correct_request_headers(delete_mock.calls[0].request)

responses.get(url, status=401)
with pytest.raises(HTTPError):
resp = client.get("/octocat")
resp.raise_for_status()


def assert_correct_request_headers(request: requests.Request):
assert (
request.headers["accept"] == "application/vnd.github+json"
), "Incorrect Accept in request to GitHub REST API"
assert (
request.headers["authorization"] == "Bearer abc"
), "Incorrect Authorization in request to GitHub REST API"


def test_sync_client_retries_on_5xx(responses, sync_client):
client = sync_client
url = f"{GITHUB_API_ENDPOINT}/octocat"
Expand Down