8000 Implement `sendEach` and `sendEachForMulticast` by Doris-Ge · Pull Request #2097 · firebase/firebase-admin-node · GitHub
[go: up one dir, main page]

Skip to content

Conversation

Doris-Ge
Copy link
Contributor
@Doris-Ge Doris-Ge commented Mar 3, 2023

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.

`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 Doris-Ge requested a review from lahirumaramba March 3, 2023 18:14
* Sends each message in the given array via Firebase Cloud Messaging.
*
* Unlike {@link Messaging.sendAll}, this method makes a single RPC call for each message in the given array.
* Unlike {@link Messaging.sendAll}, this method makes a single RPC call for each message
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this be a RPC call or an HTTP call?

Copy link
Member

Choose a reason for hiding this comment

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

I think we use RPC in other places in the SDK. Let's get the docs reviewed so we can address all these naming issues. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good! Based on the implementation, I think we actually send an HTTP call instead of an internal RPC call. I can leave this as it is now.

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 @Doris-Ge ! Looks great!

Let's get the docs reviewed. Feel free to do it now or later when you make the merge to the main branch. Thanks!

@Doris-Ge Doris-Ge merged commit a0a02f2 into fcm-batch-send Mar 30, 2023
Doris-Ge added a commit that referenced this pull request Apr 12, 2023
…nd (#2138)

* Deprecate sendAll and sendMulticast (#2094)

1. Deprecate sendAll and sendMulticast
2. Add dummy implementation for sendEach and sendEachForMulticast to avoid errors reported by api-extractor

* Implement `sendEach` and `sendEachForMulticast` (#2097)

`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.

* Add integration tests for `sendEach` and `sendMulticast` (#2130)

* Avoid using "-- i.e." in the function comment
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
0