Skip to content

fix(fcm): reject empty message batches before dispatch #986

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
Shubham-Padkonde wants to merge 2 commits into
base: main
Choose a base branch
from
+38 −4
Open

fix(fcm): reject empty message batches before dispatch #986

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
ebf38c5
fix(fcm): reject empty message batches before dispatch
Shubham-Padkonde Sep 22, 2026
ebf718a
fix(fcm): report empty multicast recipients clearly
Shubham-Padkonde Sep 22, 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
16 changes: 12 additions & 4 deletions firebase_admin/messaging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def send_each(
recipients. Instead, FCM performs all the usual validations and emulates the send operation.

Args:
messages: A list of ``messaging.Message`` instances.
messages: A non-empty list of up to 500 ``messaging.Message`` instances.
dry_run: A boolean indicating whether to run the operation in dry run mode (optional).
app: An App instance (optional).

Expand All @@ -160,7 +160,7 @@ async def send_each_async(
recipients. Instead, FCM performs all the usual validations and emulates the send operation.

Args:
messages: A list of ``messaging.Message`` instances.
messages: A non-empty list of up to 500 ``messaging.Message`` instances.
dry_run: A boolean indicating whether to run the operation in dry run mode (optional).
app: An App instance (optional).

Expand Down Expand Up @@ -203,6 +203,8 @@ def _get_messages_from_multicast(multicast_message: MulticastMessage) -> List[Me
fid=fid
) for fid in multicast_message.fids])

if not messages:
raise ValueError('multicast_message must contain at least one token or fid.')
return messages

async def send_each_for_multicast_async(
Expand All @@ -217,7 +219,8 @@ async def send_each_for_multicast_async(
recipients. Instead, FCM performs all the usual validations and emulates the send operation.

Args:
multicast_message: An instance of ``messaging.MulticastMessage``.
multicast_message: An instance of ``messaging.MulticastMessage`` with at least one
token or fid.
dry_run: A boolean indicating whether to run the operation in dry run mode (optional).
app: An App instance (optional).

Expand All @@ -238,7 +241,8 @@ def send_each_for_multicast(multicast_message, dry_run=False, app=None):
recipients. Instead, FCM performs all the usual validations and emulates the send operation.

Args:
multicast_message: An instance of ``messaging.MulticastMessage``.
multicast_message: An instance of ``messaging.MulticastMessage`` with at least one
token or fid.
dry_run: A boolean indicating whether to run the operation in dry run mode (optional).
app: An App instance (optional).

Expand Down Expand Up @@ -445,6 +449,8 @@ def send_each(self, messages: List[Message], dry_run: bool = False) -> BatchResp
"""Sends the given messages to FCM via the FCM v1 API."""
if not isinstance(messages, list):
raise ValueError('messages must be a list of messaging.Message instances.')
if not messages:
raise ValueError('messages must not be empty.')
Comment on lines +452 to +453

@gemini-code-assist gemini-code-assist Bot Sep 22, 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.

medium

Raising ValueError('messages must not be empty.') here results in a leaky abstraction when called via send_each_for_multicast or send_each_for_multicast_async. If a user passes a MulticastMessage with empty tokens/fids, they will receive an error message referring to messages, which is an internal parameter they did not directly provide.\n\nTo provide a cleaner API experience, consider validating that the MulticastMessage contains at least one token or fid inside _get_messages_from_multicast instead, and raising a more appropriate error message.\n\nFor example, in _get_messages_from_multicast:\npython\ndef _get_messages_from_multicast(multicast_message: MulticastMessage) -> List[Message]:\n # ... existing extraction logic ...\n if not messages:\n raise ValueError('multicast_message must contain at least one token or fid.')\n return messages\n

if len(messages) > 500:
raise ValueError('messages must not contain more than 500 elements.')

Expand Down Expand Up @@ -473,6 +479,8 @@ async def send_each_async(self, messages: List[Message], dry_run: bool = True) -
"""Sends the given messages to FCM via the FCM v1 API."""
if not isinstance(messages, list):
raise ValueError('messages must be a list of messaging.Message instances.')
if not messages:
raise ValueError('messages must not be empty.')
if len(messages) > 500:
raise ValueError('messages must not contain more than 500 elements.')

Expand Down
26 changes: 26 additions & 0 deletions tests/test_messaging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1953,6 +1953,15 @@ def test_invalid_send_each(self, msg):
expected = 'messages must be a list of messaging.Message instances.'
assert str(excinfo.value) == expected

def test_send_each_empty_batch(self):
with pytest.raises(ValueError, match='messages must not be empty.'):
messaging.send_each([])

@pytest.mark.asyncio
async def test_send_each_async_empty_batch(self):
with pytest.raises(ValueError, match='messages must not be empty.'):
await messaging.send_each_async([])

def test_invalid_over_500(self):
msg = messaging.Message(topic='foo')
with pytest.raises(ValueError) as excinfo:
Expand Down Expand Up @@ -2273,6 +2282,23 @@ def test_invalid_send_each_for_multicast(self, msg):
expected = 'Message must be an instance of messaging.MulticastMessage class.'
assert str(excinfo.value) == expected

@pytest.mark.parametrize('recipients', [{'tokens': []}, {'fids': []},
{'tokens': [], 'fids': []}])
def test_send_each_for_multicast_empty_batch(self, recipients):
msg = messaging.MulticastMessage(**recipients)
with pytest.raises(
ValueError, match='multicast_message must contain at least one token or fid.'):
messaging.send_each_for_multicast(msg)

@pytest.mark.asyncio
@pytest.mark.parametrize('recipients', [{'tokens': []}, {'fids': []},
{'tokens': [], 'fids': []}])
async def test_send_each_for_multicast_async_empty_batch(self, recipients):
msg = messaging.MulticastMessage(**recipients)
with pytest.raises(
ValueError, match='multicast_message must contain at least one token or fid.'):
await messaging.send_each_for_multicast_async(msg)

def test_send_each_for_multicast(self):
payload1 = json.dumps({'name': 'message-id1'})
payload2 = json.dumps({'name': 'message-id2'})
Expand Down
Loading