Skip to content

fix(fcm): encode durations without floating-point rounding errors - #993

Open
Shubham-Padkonde wants to merge 2 commits into
firebase:mainfrom
Shubham-Padkonde:fix/fcm-duration-precision
Open

Shubham-Padkonde wants to merge 2 commits into
firebase:mainfrom
Shubham-Padkonde:fix/fcm-duration-precision

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown

encode_ttl and encode_milliseconds computed the nanoseconds from the float returned by timedelta.total_seconds(), so some durations were sent with rounding errors: an AndroidConfig.ttl of 86400.9 seconds (or timedelta(days=1, microseconds=900000)) was encoded as "86400.899999999s", and 1.123457 as "1.123456999s". The Duration string is now built from the exact days/seconds/microseconds of the timedelta.

Hey there! So you want to contribute to a Firebase SDK?
Before you file this pull request, please read these guidelines:

Discussion

  • Read the contribution guidelines (CONTRIBUTING.md).
  • If this has been discussed in an issue, make sure to link to the issue here.
    If not, go file an issue about this before creating a pull request to discuss.

Testing

  • Make sure all existing tests in the repository pass after your change.
  • If you fixed a bug or added a feature, add a new test to cover your code.

API Changes

  • At this time we cannot accept changes that affect the public API. If you'd like to help
    us make Firebase APIs better, please propose your change in an issue so that we
    can discuss it together.

encode_ttl and encode_milliseconds computed the nanoseconds from the
float returned by timedelta.total_seconds(), so some durations were
sent with rounding errors: an AndroidConfig.ttl of 86400.9 seconds (or
timedelta(days=1, microseconds=900000)) was encoded as
"86400.899999999s", and 1.123457 as "1.123456999s". The Duration
string is now built from the exact days/seconds/microseconds of the
timedelta.
@Shubham-Padkonde
Shubham-Padkonde requested a review from a team September 25, 2026 15:44

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

Copy link
Copy Markdown

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 refactors duration encoding in firebase_admin/_messaging_encoder.py by introducing an encode_duration helper method. This method uses exact integer fields of datetime.timedelta to avoid floating-point precision issues associated with total_seconds(). Corresponding test cases have been added to tests/test_messaging.py. The feedback suggests using Python's f-string integer formatting {nanos:09d} instead of str(nanos).zfill(9) for cleaner and more idiomatic code.

Comment thread firebase_admin/_messaging_encoder.py Outdated
Comment on lines 302 to 303
if nanos:
return f'{seconds}.{str(nanos).zfill(9)}s'

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

Using Python's f-string integer formatting {nanos:09d} is more idiomatic, cleaner, and slightly more efficient than converting to a string and calling zfill(9).

Suggested change
if nanos:
return f'{seconds}.{str(nanos).zfill(9)}s'
if nanos:
return f'{seconds}.{nanos:09d}s'

This branch has not been deployed

No deployments
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.

1 participant