8000 feat(fcm): Add `sendEach` and `sendEachForMulticast` for FCM batch send by Doris-Ge · Pull Request #2138 · firebase/firebase-admin-node · GitHub
[go: up one dir, main page]
Skip to content

feat(fcm): Add sendEach and sendEachForMulticast for FCM batch send #2138

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

Jump to bottom
Merged
merged 5 commits into from
Apr 12, 2023
Merged

feat(fcm): Add sendEach and sendEachForMulticast for FCM batch send #2138

merged 5 commits into from
Apr 12, 2023

Conversation

Doris-Ge
Copy link
Contributor
@Doris-Ge Doris-Ge commented Apr 5, 2023 •
edited by lahirumaramba
Loading
  • Deprecate sendAll() and sendMulticast() APIs.
  • Add new sendEach() and sendEachForMulticast() APIs.

RELEASE NOTE: sendAll() and sendMulticast() APIs are now deprecated. Please use sendEach() and sendEachForMulticast() APIs instead.

Doris-Ge added 3 commits March 3, 2023 09:41
@Doris-Ge
Deprecate sendAll and sendMulticast (#2094)
2725f8c 
1. Deprecate sendAll and sendMulticast
2. Add dummy implementation for sendEach and sendEachForMulticast to avoid errors reported by api-extractor
@Doris-Ge
Implement sendEach and sendEachForMulticast (#2097)
a0a02f2 
`sendEach` vs `sendAll`
1. `sendEach` sends one HTTP request to V1 Send endpoint for each
    message in the array.
   `sendAll` sends only one HTTP request to V1 Batch Send endpoint
    to send all messages in the array.
2. `sendEach` calls `Promise.allSettled` to wait for all
   `httpClient.send` calls to complete and construct a `BatchResponse`.
    An `httpClient.send` call to V1 Send endpoint either completes
    with a success or throws an error. So if an error is thrown out,
    the error will be caught in `sendEach` and turned into a
    `SendResponse` with an error.
    Therefore, unlike `sendAll`, `sendEach` does not always throw
    an error for a total failure. It can also return a `BatchResponse`
    with only errors in it.

`sendEachForMulticast` calls `sendEach` under the hood.
@Doris-Ge
Add integration tests for sendEach and sendMulticast (#2130)
6fa3c00
@Doris-Ge Doris-Ge requested a review from lahirumaramba April 5, 2023 19:32
@lahirumaramba lahirumaramba changed the title sendEach and sendEachForMulticast for FCM batch send feat(fcm): Add sendEach and sendEachForMulticast for FCM batch send Apr 5, 2023
@lahirumaramba lahirumaramba added release-note release:stage Stage a release candidate labels Apr 5, 2023
@lahirumaramba
Copy link
Member

Thank you @Doris-Ge . Let's get the docs reviewed by @egilmorez before we merge this.

@lahirumaramba lahirumaramba requested a review from egilmorez April 11, 2023 18:06
egilmorez
egilmorez approved these changes Apr 11, 2023
View reviewed changes
Copy link
Contributor
@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

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

LG with one optional nit, thanks!

We avoid i. e., so for this I'd suggest just "...indicates a total failure, meaning that none of the messages in the list could be sent.

@Doris-Ge
Copy link
Contributor Author

Thanks Eric for reviewing the PR! @lahirumaramba I tried to push the fix to the fcm-batch-send branch but it got rejected because "Protected branch update failed for refs/heads/fcm-batch-send". Should I create another PR from dorisge/fcm-batch-send for the fix and merge it to fcm-batch-send?

lahirumaramba
lahirumaramba approved these changes Apr 12, 2023
View reviewed changes
Copy link
Member
@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM

@Doris-Ge Doris-Ge merged commit 0da72ef into master Apr 12, 2023
@Doris-Ge Doris-Ge deleted the fcm-batch-send branch April 12, 2023 19:21
@vigneshgurusamy vigneshgurusamy mentioned this pull request Jun 21, 2023
Closed
@roblo1801 roblo1801 mentioned this pull request May 17, 2024
Open
@X-oss-byte X-oss-byte mentioned this pull request Jun 11, 2024
Open
This was referenced Aug 20, 2024
Closed
Closed
Closed
This was referenced Aug 28, 2024
Closed
Closed
Closed
Closed
This was referenced Sep 4, 2024
Closed
Closed
@jcharra jcharra mentioned this pull request Sep 24, 2024
Closed
This was referenced Sep 25, 2024
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@egilmorez egilmorez egilmorez approved these changes

@lahirumaramba lahirumaramba lahirumaramba approved these changes

Assignees

@egilmorez egilmorez

Labels
release:stage Stage a release candidate release-note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Doris-Ge @lahirumaramba @egilmorez
0