Skip to content

fix(pubsub): accept messages without data - #322

Open
IzaakGough wants to merge 4 commits into
mainfrom
fix/pubsub-message-without-data
Open

IzaakGough wants to merge 4 commits into
mainfrom
fix/pubsub-message-without-data

Conversation

@IzaakGough

@IzaakGough IzaakGough commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

A Pub/Sub message that has attributes but no data crashed on_message_published with TypeError: Message.__init__() missing 1 required positional argument: 'data' before the handler ran.

This lets those messages through with data set to None, makes None the default for data when building a Message directly, and types publish_time as the datetime it already is.

  • ordering_key is now str | None = None. The field after data needs a default too, and the handler already passed None when there is no ordering key.
  • data is now typed str | None, so type checkers will flag code that passes msg.data straight to b64decode. .json returns None for both a missing and an empty payload, because the emulator sends "" for attribute-only messages.
  • Draft refactor: replace datetime parsing with utility function for consistency across event handlers #274 removes the datetime import this annotation uses. Whichever PR lands second should keep it.

Tested with unit tests and in the emulator with an attribute-only message. Not tested with a real Pub/Sub delivery.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request updates the Message class in pubsub_fn.py to change the type of publish_time to a timezone-aware datetime and allows data to be None when a Pub/Sub message is published without a payload. It also updates the message handler to default data to None and adds comprehensive unit tests to verify these behaviors. I have no feedback to provide as there are no review comments.

@IzaakGough
IzaakGough marked this pull request as ready for review October 22, 2026 13:47

@CorieW CorieW left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

nits:

  • data="" still makes .json raise, while data=None returns None. Should "" return None too, in case the emulator sends attribute-only messages that way?
  • data: str | None may cause new mypy/pyright warnings for users (e.g. b64decode(msg.data)). Worth a changelog note?
  • No test covers a present orderingKey (e.g. "abc" → ordering_key == "abc").
  • The older tests could reuse _raw_event instead of building the CloudEvent by hand.

The emulator sends data as an empty string for attribute-only messages.
Also cover a present ordering key and reuse _raw_event in older tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants