Skip to content
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.

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

messaging.sendMulticast, sendAll rejects valid ttl #479

Closed
stari4ek opened this issue Mar 16, 2019 · 4 comments
Closed

messaging.sendMulticast, sendAll rejects valid ttl #479

stari4ek opened this issue Mar 16, 2019 · 4 comments

Comments

@stari4ek
Copy link

stari4ek commented Mar 16, 2019

"firebase-admin": "^7.1.0",

When I'm sending message with ttl it got rejected while it's totally valid (number, >0)

const msg: firebase.messaging.MulticastMessage = {
    tokens: tokens,
    data: data,
    android: {
        ttl: 1000000
    }
};
console.log('Sending\n', msg);
const batchResponse: firebase.messaging.BatchResponse = await firebase.messaging().sendMulticast(msg, DRY_RUN);

In firebase log:

Sending
 { tokens: 
   [ SKIPPED],
  data: 
   { SKIPPED },
  android: { ttl: 1000000 } }

{ Error: TTL must be a non-negative duration in milliseconds
    at FirebaseMessagingError.FirebaseError [as constructor] (/srv/node_modules/firebase-admin/lib/utils/error.js:42:28)
    at FirebaseMessagingError.PrefixedFirebaseError [as constructor] (/srv/node_modules/firebase-admin/lib/utils/error.js:88:28)
    at new FirebaseMessagingError (/srv/node_modules/firebase-admin/lib/utils/error.js:253:16)
    at validateAndroidConfig (/srv/node_modules/firebase-admin/lib/messaging/messaging-types.js:249:19)
    at Object.validateMessage (/srv/node_modules/firebase-admin/lib/messaging/messaging-types.js:49:5)
    at /srv/node_modules/firebase-admin/lib/messaging/messaging.js:260:31
    at Array.map (<anonymous>)
    at Messaging.sendAll (/srv/node_modules/firebase-admin/lib/messaging/messaging.js:259:33)
    at Messaging.sendMulticast (/srv/node_modules/firebase-admin/lib/messaging/messaging.js:308:21)
    at Object.<anonymous> (/srv/lib/controller/fcm.js:79:62)
  errorInfo: 
   { code: 'messaging/invalid-payload',
     message: 'TTL must be a non-negative duration in milliseconds' },
  codePrefix: 'messaging' }

Source code looks totally valid:

        if (!validator.isNumber(config.ttl) || config.ttl < 0) {
            throw new error_1.FirebaseMessagingError(error_1.MessagingClientErrorCode.INVALID_PAYLOAD, 'TTL must be a non-negative duration in milliseconds');
        }

but it rejects proper ttl.
Other configs like collapseKey work well

Ps. there is unnecessary deepCopy in sendAll

@google-oss-bot
Copy link

I found a few problems with this issue:

  • I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
  • This issue does not seem to follow the issue template. Make sure you provide all the required information.

@stari4ek stari4ek changed the title sendMulticast rejects valid ttl messaging.sendMulticast rejects valid ttl Mar 16, 2019
@stari4ek
Copy link
Author

stari4ek commented Mar 16, 2019

ok.
most probably issue is with validator (validateAndroidConfig) which mutates tts field to string.
so if messages share single AndroidConfig object - it will fail.

so, fixing those half-used deepCopy should fix this issue

@stari4ek stari4ek changed the title messaging.sendMulticast rejects valid ttl messaging.sendMulticast, sendAll rejects valid ttl Mar 16, 2019
@hiranya911
Copy link
Contributor

Thanks @stari4ek for reporting this. And good diagnosis. I'll try to get this fixed for the next release. Beware that there are probably other parameters that are affected by the same bug.

@tominou
Copy link

tominou commented Jan 31, 2021

The issue still appears in 9.4.2 version using .messaging().sendMulticast

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants