ToothlessGear / node-gcm

A NodeJS wrapper library port to send data to Android devices via Google Cloud Messaging
https://github.com/ToothlessGear/node-gcm
Other
1.3k stars 206 forks source link

Support for sending to > 1,000 registration tokens #167

Open eladnava opened 8 years ago

eladnava commented 8 years ago

As discussed in #42 with @hypesystem, we'd like to be able to support more than 1,000 registration tokens in the sender.send and sender.sendNoRetry calls -- today, sending more than 1,000 registration tokens will invoke an error, as GCM will reject the request.

Modules

eladnava commented 8 years ago

@hypesystem did you get a chance to take a look at this, mate?

hypesystem commented 8 years ago

Not yet. The biggest challenge, as I see it, is if some batches succeed and other fails: this needs to be returned in a meaningful way to the user.

I'll give an example with some smaller sizes: we want to send 6 notifications and have a batch size of 2. This happens:

How do we represent the result here?

We cannot just return an error, because some of the notifications were sent. The user needs to see this, and not resend them.

The best suggestion I have is to return success and a result object looking something like this:

[
  { success: true },
  { success: true },
  { success: true },
  { error: "original error that happened" },
  { error: "some error we made up for 500-codes" },
  { error: "some error we made up for 500-codes" }
]

For failing batches we return a result-object for each notification that explains the error.

Again, this is my best suggestion at the moment, and there might be a better solution. No matter what it is a tricky situation to handle.

eladnava commented 8 years ago

@hypesystem what do you mean by notification?

For failing batches we return a result-object for each notification that explains the error.

Are you referring to each individual notification sent to every device in the current batch?

hypesystem commented 8 years ago

By notification I mean the individual message sent to a device. I should have probably just called it "message" to lessen confusion.

Are you referring to each individual notification sent to every device in the current batch?

Yes. And also the final response when merging all the results before returning the result to the user.

eladnava commented 8 years ago

@hypesystem OK, that makes sense.

This is the format of the current result object returned in the sender.send callback:

{
    "failure" : 1, // Number of messages that could not be processed
    "success" : 5, // Number of messages that were sent successfully
    "canonical_ids": [...],
    "results": [...],
    ...
}

Given that apps already interface with it, I think that we should not modify its format. Batch requests should return the same object, incrementing the failure count or erroring out completely if necessary. And they could concat the results and canonical_ids from all of the batch requests into that single array which is returned to the app, in the same order that the registration tokens were supplied.

GCM Error Response Handling for Batch Requests

I think that we need to define what "batch request fails" means. There are 4 types of possible responses from the GCM downstream HTTP API.

This is how I think we should handle the HTTP response codes for each batch request:

As per your example, the result returned should be:

{
    "failure" : 1, // Number of messages that could not be processed
    "success" : 5, // Number of messages that were sent successfully
    "canonical_ids": [...],
    "results": [...],
    ....
}

What do you think?

rraziel commented 8 years ago

Checking the error responses is nice but what if the request times out? Should it be considered like a failure, even though the messages may have gone through? I suppose the same issue exists for submissions <= 1000 requests as well though.

hypesystem commented 8 years ago

I think a timeout should just result in a retry; if it times out all the tries, it will fail altogether for all the affected registration ids.

BackPackerDz commented 7 years ago

Hi, it's not available ?

eladnava commented 7 years ago

@BackPackerDz No, it is not. Check out a workaround here: https://github.com/ToothlessGear/node-gcm/issues/42#issuecomment-145379922