Skip to content

Introduce Accept headers for sending federation transactions #36

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Introduce Accept headers for sending federation transactions #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions synapse/federation/transport/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,8 @@ async def send_transaction(

path = _create_v1_path("/send/%s", transaction.transaction_id)

headers: Dict[bytes, List[bytes]] = {b"Accept": [b"application/json"]}

return await self.client.put_json(
transaction.destination,
path=path,
Expand All @@ -262,6 +264,7 @@ async def send_transaction(
# Sending a transaction should always succeed, if it doesn't
# then something is wrong and we should backoff.
backoff_on_all_error_codes=True,
headers=headers,
)

async def make_query(
Expand Down
27 changes: 21 additions & 6 deletions synapse/http/matrixfederationclient.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,13 +532,15 @@ async def _send_request_with_optional_trailing_slash(
async def _send_request(
self,
request: MatrixFederationRequest,
*,
retry_on_dns_fail: bool = True,
timeout: Optional[int] = None,
long_retries: bool = False,
ignore_backoff: bool = False,
backoff_on_404: bool = False,
backoff_on_all_error_codes: bool = False,
follow_redirects: bool = False,
headers: Optional[Dict[bytes, List[bytes]]] = None,
Copy link
Member

@nico-famedly nico-famedly May 13, 2025

Choose a reason for hiding this comment

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

I don't think this should be a dictionary of generic headers, we really just want to have the expected mimetype for a request, imo

Copy link
Contributor Author

@jason-famedly jason-famedly May 13, 2025

Choose a reason for hiding this comment

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

It was based on prior art, which allowed for a very simple way of rolling this into the existing headers with a simple update. What are the chances of needing some other kind of header later on from some other endpoint needing it? Like media? I would have liked to be able to add the Content-Length or something like it there(if I recall, it was missing since the last change?)

) -> IResponse:
"""
Sends a request to the given server.
Expand Down Expand Up @@ -584,6 +586,9 @@ async def _send_request(
follow_redirects: True to follow the Location header of 307/308 redirect
responses. This does not recurse.

headers: Additional Headers to pass to the request. Authorization,
Content-type, User-Agent and Host will be overridden below

Returns:
Resolves with the HTTP response object on success.

Expand Down Expand Up @@ -646,6 +651,8 @@ async def _send_request(

# Inject the span into the headers
headers_dict: Dict[bytes, List[bytes]] = {}
if headers:
headers_dict.update(headers)
opentracing.inject_header_dict(headers_dict, request.destination)

headers_dict[b"User-Agent"] = [self.version_string_bytes]
Expand Down Expand Up @@ -754,14 +761,15 @@ async def _send_request(

return await self._send_request(
attr.evolve(request, uri=new_uri, generate_uri=False),
retry_on_dns_fail,
timeout,
long_retries,
ignore_backoff,
backoff_on_404,
backoff_on_all_error_codes,
retry_on_dns_fail=retry_on_dns_fail,
timeout=timeout,
long_retries=long_retries,
ignore_backoff=ignore_backoff,
backoff_on_404=backoff_on_404,
backoff_on_all_error_codes=backoff_on_all_error_codes,
# Do not continue following redirects.
follow_redirects=False,
headers=headers,
)
else:
logger.info(
Expand Down Expand Up @@ -942,6 +950,7 @@ async def put_json(
try_trailing_slash_on_400: bool = False,
parser: Literal[None] = None,
backoff_on_all_error_codes: bool = False,
headers: Optional[Dict[bytes, List[bytes]]] = None,
Copy link
Member

@nico-famedly nico-famedly May 13, 2025

Choose a reason for hiding this comment

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

This should not default to None, we are expecting json...

Copy link
Contributor Author

@jason-famedly jason-famedly May 13, 2025

Choose a reason for hiding this comment

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

We are expecting json, but we also do not know which places this is safe to inject into without other repercussions. It's optional for a reason, and matches well with other pre-existing similar architecture that is used

) -> JsonDict: ...

@overload
Expand All @@ -959,6 +968,7 @@ async def put_json(
try_trailing_slash_on_400: bool = False,
parser: Optional[ByteParser[T]] = None,
backoff_on_all_error_codes: bool = False,
headers: Optional[Dict[bytes, List[bytes]]] = None,
) -> T: ...

async def put_json(
Expand All @@ -975,6 +985,7 @@ async def put_json(
try_trailing_slash_on_400: bool = False,
parser: Optional[ByteParser[T]] = None,
backoff_on_all_error_codes: bool = False,
headers: Optional[Dict[bytes, List[bytes]]] = None,
) -> Union[JsonDict, T]:
"""Sends the specified json data using PUT

Expand Down Expand Up @@ -1012,6 +1023,9 @@ async def put_json(
parsing as JSON.
backoff_on_all_error_codes: Back off if we get any error response

headers: Additional Headers to pass to the request. Authorization,
Content-type, User-Agent and Host will be overridden below

Returns:
Succeeds when we get a 2xx HTTP response. The
result will be the decoded JSON body.
Expand Down Expand Up @@ -1045,6 +1059,7 @@ async def put_json(
long_retries=long_retries,
timeout=timeout,
backoff_on_all_error_codes=backoff_on_all_error_codes,
headers=headers,
)

if timeout is not None:
Expand Down
Loading