fix(bedrock): use safe dict comprehension to remove connection header - #1352
Open
FuturizeRush wants to merge 4 commits into
Open
fix(bedrock): use safe dict comprehension to remove connection header#1352FuturizeRush wants to merge 4 commits into
FuturizeRush wants to merge 4 commits into
Conversation
`del headers["connection"]` raises KeyError when the header is absent (e.g. HTTP/2 connections or certain proxy configurations). The equivalent code in lib/aws/_auth.py already uses a safe dict comprehension. This aligns the bedrock version with the same pattern.
karpetrosyan
reviewed
Apr 9, 2026
| # that are signed. | ||
| headers = headers.copy() | ||
| del headers["connection"] | ||
| headers = {k: v for k, v in dict(headers).items() if k.lower() != "connection"} |
Contributor
There was a problem hiding this comment.
Suggested change
| headers = {k: v for k, v in dict(headers).items() if k.lower() != "connection"} | |
| headers = {k: v for k, v in headers.items() if k.lower() != "connection"} |
Contributor
There was a problem hiding this comment.
CI fails because it expects headers to be an instance of Headers.
We can just keep the original headers.copy() but instead of del headers["connection"] do headers.pop("connection", None)
added 3 commits
April 10, 2026 12:32
Fixes pyright reportAssignmentType error -- dict comprehension returns dict[str, str] which is not assignable to httpx.Headers. Use new_headers variable instead, matching the pattern in lib/aws/_auth.py.
Simplify by reusing the headers variable name instead of introducing new_headers, reducing the diff to a single line change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace
del headers["connection"]with a dict comprehension that safely filters out the header, preventingKeyErrorwhen the header is absent.Bug
lib/bedrock/_auth.py:60:The equivalent code in
lib/aws/_auth.py:60already handles this safely:The bedrock version crashes when
connectionheader is missing, which happens with:connectionheader by spec)Fix
Align bedrock's implementation with the aws version — one line, same dict comprehension pattern.
Test plan
connectionheader exists: filtered out (same behavior as before)connectionheader is absent: no error (fixed).lower()(matches aws version)