arrays of arrays

[jamesholcomb]

fcm-v1-http2/index.js

Line 62 in 8506b6e

unregisteredTokens.push(unregisteredTokensList);

fcm-v1-http2/index.js

Line 37 in 8506b6e

tokenBatches.push(tokens.slice(start, start + batchLimit));

you need to spread the input arg...TS would have caught this type of error.

[eladnava]

Hi @jamesholcomb,
Thanks for catching that!

Indeed, the unregisteredTokens array should only contain device token strings, not arrays of device token strings. Fixed in 5a0266c and published to npm in 1.0.5.

As per line number 37:

fcm-v1-http2/index.js

Line 37 in 8506b6e

tokenBatches.push(tokens.slice(start, start + batchLimit));

The variable tokenBatches is supposed to contain an array of token batches, so no spread operator is needed there.

Please let me know if there's anything else I can help with.

[jamesholcomb]

can you remove the console.error call and instead allow a Logger impl with the standard Console function signatures to be passed to the Client? it would be helpful with debugging issues in production since testing extreme loads with real device tokens is basically impossible.

[eladnava]

Hi @jamesholcomb,
You are welcome to submit a PR with the requested changes.

[jamesholcomb]

indeed i considered this, but it's too impractical to maintain this JS code. also, there are no unit tests, no linting, no prettier rules,...it's not setup for community driven changes.

[fctaddia]

Hello in the TS project that I'm keeping (https://gitlab.com/kenble/fcm-http2), always thanking eladnava, this fix was already present. This is because I had noticed this problem from my tests.
@jamesholcomb since you're opening issues to both me and eladnava, I don't understand what the problem is. Also in this issue you talk about extreme load... In issue (https://gitlab.com/kenble/fcm-http2/-/issues/2) that you opened to me you talk about an analysis of the extreme load. I would like to understand your needs and in case both eladnava and I could help you.
Thank you

[jamesholcomb]

I ended up switching to firebase-admin once they implemented http2 sessions.

[eladnava]

Please be advised that while firebase-admin now implements HTTP/2 sessions for sendEach() and sendEachForMulitcast() in version v12.3.0 and newer, there is no HTTP/2 multiplexing support, so notifications are sent over a single HTTP/2 connection, which is limited to 100 simultaneous streams (requests).

This means that only 100 notifications can be sent at any given time over this single connection, which means that firebase-admin is not suitable for use cases requiring high throughput (> 100 notifications per second).

For high-throughput, low-latency notification delivery, it is still recommended to make use of a solution which employs HTTP/2 multiplexing, such as this package.