Skip to content

Conversation

@Doris-Ge
Copy link
Contributor

SendEachAsync vs SendAllAsync

  1. SendEachAsync sends one HTTP request to V1 Send endpoint for each message in the list. SendAllAsync sends only one HTTP request to V1 Batch Send endpoint to send all messages in the list.
  2. SendEachAsync calls Task.WhenAll to wait for all httpClient.SendAndDeserializeAsync calls to complete and construct a BatchResponse with all SendResponses. An httpClient.SendAndDeserializeAsync call to V1 Send endpoint either completes with a success or throws an exception. So if an exception is thrown out, the exception will be caught in SendEachAsync and turned into a SendResponse with an exception. Therefore, unlike SendAllAsync, SendEachAsync does not always throw an exception for a total failure. It can also return a BatchResponse with only exceptions in it.

SendEachForMulticastAsync calls SendEachAsync under the hood.

`SendEachAsync` vs `SendAllAsync`
1. `SendEachAsync` sends one HTTP request to V1 Send endpoint for each
    message in the list.
   `SendAllAsync` sends only one HTTP request to V1 Batch Send endpoint
    to send all messages in the list.
2. `SendEachAsync` calls `Task.WhenAll` to wait for all
    `httpClient.SendAndDeserializeAsync` calls to complete and construct
    a `BatchResponse` with all `SendResponse`s.
    An `httpClient.SendAndDeserializeAsync` call to V1 Send endpoint
    either completes with a success or throws an exception. So if an
    exception is thrown out, the exception will be caught in `SendEachAsync`
    and turned into a `SendResponse` with an exception.
    Therefore, unlike `SendAllAsync`, `SendEachAsync` does not always throw
    an exception for a total failure. It can also return a `BatchResponse`
    with only exceptions in it.

`SendEachForMulticastAsync` calls `SendEachAsync` under the hood.
@Doris-Ge Doris-Ge force-pushed the dorisge/fcm-batch-send branch from 27e6ee1 to 6d6b20f Compare March 29, 2023 06:34
}
catch (Exception e)
{
var exception = new FirebaseMessagingException(ErrorCode.Unknown, $"{e}");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it ok to put the exception into the SendResponse?

Copy link
Member

Choose a reason for hiding this comment

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

Seems like that's how we have handled exceptions in batch send. Add the responses in var responses = new List<SendResponse>(); with SendResponse.FromException(... for failed once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the late response! My question was about whether it's ok to put the content of the Exception e into the SendResponse via $"{e}". Sorry for being unclear!

@Doris-Ge Doris-Ge requested a review from lahirumaramba March 29, 2023 16:51
SendEach() and SendEachForMulticast()
@lahirumaramba lahirumaramba added the release:stage Stage a release candidate label Jun 1, 2023
@lahirumaramba lahirumaramba changed the title Implement SendEachAsync and SendEachForMulticastAsync feat(fcm): Implement SendEachAsync and SendEachForMulticastAsync Jun 1, 2023
@lahirumaramba lahirumaramba added release-note and removed release-note release:stage Stage a release candidate labels Jun 1, 2023
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.

LGTM! Thank you!

}
catch (Exception e)
{
var exception = new FirebaseMessagingException(ErrorCode.Unknown, $"{e}");
Copy link
Member

Choose a reason for hiding this comment

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

Seems like that's how we have handled exceptions in batch send. Add the responses in var responses = new List<SendResponse>(); with SendResponse.FromException(... for failed once.

@Doris-Ge Doris-Ge merged commit 51dcefe into fcm-batch-send Jun 8, 2023
Doris-Ge added a commit that referenced this pull request Jun 9, 2023
…M batch send (#348)

* feat(fcm): Implement `SendEachAsync` and `SendEachForMulticastAsync` (#343)

* Implement `SendEachAsync` and `SendEachForMulticastAsync`

`SendEachAsync` vs `SendAllAsync`
1. `SendEachAsync` sends one HTTP request to V1 Send endpoint for each
    message in the list.
   `SendAllAsync` sends only one HTTP request to V1 Batch Send endpoint
    to send all messages in the list.
2. `SendEachAsync` calls `Task.WhenAll` to wait for all
    `httpClient.SendAndDeserializeAsync` calls to complete and construct
    a `BatchResponse` with all `SendResponse`s.
    An `httpClient.SendAndDeserializeAsync` call to V1 Send endpoint
    either completes with a success or throws an exception. So if an
    exception is thrown out, the exception will be caught in `SendEachAsync`
    and turned into a `SendResponse` with an exception.
    Therefore, unlike `SendAllAsync`, `SendEachAsync` does not always throw
    an exception for a total failure. It can also return a `BatchResponse`
    with only exceptions in it.

`SendEachForMulticastAsync` calls `SendEachAsync` under the hood.

* Add integration tests for the batch-send reimplementation:
SendEach() and SendEachForMulticast()

* Address the doc review comments
Doris-Ge added a commit that referenced this pull request Jul 25, 2023
…343)

* Implement `SendEachAsync` and `SendEachForMulticastAsync`

`SendEachAsync` vs `SendAllAsync`
1. `SendEachAsync` sends one HTTP request to V1 Send endpoint for each
    message in the list.
   `SendAllAsync` sends only one HTTP request to V1 Batch Send endpoint
    to send all messages in the list.
2. `SendEachAsync` calls `Task.WhenAll` to wait for all
    `httpClient.SendAndDeserializeAsync` calls to complete and construct
    a `BatchResponse` with all `SendResponse`s.
    An `httpClient.SendAndDeserializeAsync` call to V1 Send endpoint
    either completes with a success or throws an exception. So if an
    exception is thrown out, the exception will be caught in `SendEachAsync`
    and turned into a `SendResponse` with an exception.
    Therefore, unlike `SendAllAsync`, `SendEachAsync` does not always throw
    an exception for a total failure. It can also return a `BatchResponse`
    with only exceptions in it.

`SendEachForMulticastAsync` calls `SendEachAsync` under the hood.

* Add integration tests for the batch-send reimplementation:
SendEach() and SendEachForMulticast()
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