Skip to content

fix(messaging): preserve per-message results in send_each_async #1006

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
EnesKartal wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(messaging): preserve per-message results in send_each_async #1006

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
8 changes: 8 additions & 0 deletions firebase_admin/messaging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,14 @@ async def send_data(data):
# Catch errors caused by the requests library during authorization
except requests.exceptions.RequestException as exception:
return SendResponse(resp=None, exception=self._handle_fcm_error(exception))
except Exception as exception:
return SendResponse(
resp=None,
exception=exceptions.UnknownError(
message=f'Unknown error while making remote service calls: {exception}',
cause=exception,
),
)
return SendResponse(resp.json(), exception=None)
Comment on lines +603 to 611

@gemini-code-assist gemini-code-assist Bot Nov 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

While catching unexpected exceptions during the remote call is a great improvement, calling resp.json() at line 611 is currently outside of any try-except block. If the response body is empty or contains invalid JSON, resp.json() will raise a ValueError (or JSONDecodeError), which will propagate up and abort the entire asyncio.gather() call, losing all other successful results in the batch.\n\nTo prevent this, we should wrap the resp.json() call in a try-except block as well, wrapping any parsing exceptions in an UnknownError to preserve the other results.

            except Exception as exception:\n                return SendResponse(\n                    resp=None,\n                    exception=exceptions.UnknownError(\n                        message=f'Unknown error while making remote service calls: {exception}',\n                        cause=exception,\n                    ),\n                )\n            try:\n                return SendResponse(resp.json(), exception=None)\n            except Exception as exception:\n                return SendResponse(\n                    resp=None,\n                    exception=exceptions.UnknownError(\n                        message=f'Unknown error while parsing remote service response: {exception}',\n                        cause=exception,\n                    ),\n                )


message_data = [self._message_data(message, dry_run) for message in messages]
Expand Down
28 changes: 28 additions & 0 deletions tests/test_messaging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2182,6 +2182,34 @@ async def test_send_each_async_request_error(self):
exception = batch_response.responses[0].exception
assert isinstance(exception, exceptions.UnavailableError)

@pytest.mark.asyncio
@respx.mock
async def test_send_each_async_unexpected_exception_preserves_other_results(self):
responses = [
respx.MockResponse(200, http_version='HTTP/2', json={'name': 'message-id1'}),
KeyError(4213),
]
msg1 = messaging.Message(topic='foo1')
msg2 = messaging.Message(topic='foo2')
route = respx.request(
'POST',
'https://fcm.googleapis.com/v1/projects/explicit-project-id/messages:send'
).mock(side_effect=responses)

batch_response = await messaging.send_each_async([msg1, msg2], dry_run=True)

assert route.call_count == 2
assert batch_response.success_count == 1
assert batch_response.failure_count == 1
assert len(batch_response.responses) == 2
assert batch_response.responses[0].success is True
assert batch_response.responses[0].message_id == 'message-id1'
assert batch_response.responses[1].success is False
exception = batch_response.responses[1].exception
assert isinstance(exception, exceptions.UnknownError)
assert isinstance(exception.cause, KeyError)
assert exception.cause.args == (4213,)

@pytest.mark.parametrize('status', HTTP_ERROR_CODES)
def test_send_each_detailed_error(self, status):
success_payload = json.dumps({'name': 'message-id'})
Expand Down
Loading