12

I am writing a cloud function for firebase using javascript but I am stuck, I don't know the exact meaning of error and unable to solve it.. The error states: 27:65 error Each then() should return a value or throw promise/always-return

'use strict'

const functions = require('firebase-functions');
const admin = require('firebase-admin');
admin.initializeApp();

exports.sendNotification = functions.database.ref('/notifications/{user_id}/{notification_id}').onWrite((change, context) => {

    const user_id = context.params.user_id;
    const notification_id = context.params.notification_id;
    console.log('We have a notification from : ', user_id);

    if (!change.after.val()) {
        return console.log('A Notification has been deleted from the database : ', notification_id);
    }
    const deviceToken = admin.database().ref(`/ServiceProvider/${user_id}/device_token`).once('value');
    return deviceToken.then(result => {
        const token_id = result.val();
        const payload = {
            notification: {
              title : "New Friend Request",
              body: "You Have Received A new Friend Request",
              icon: "default"
            }
        };

        return admin.messaging().sendToDevice(token_id, payload).then(response => {

            console.log('This was the notification Feature');

        });

    });

});
Doug Stevenson
  • 297,357
  • 32
  • 422
  • 441
Ikram Khan Niazi
  • 789
  • 6
  • 17
  • That doesn't sound like an error, but rather a linter warning. Specifically, the [`promise/always-return` one](https://github.com/xjamundx/eslint-plugin-promise/blob/HEAD/docs/rules/always-return.md) – Bergi Nov 17 '18 at 19:09

3 Answers3

22

Change this:

    return admin.messaging().sendToDevice(token_id, payload).then(response => {

        console.log('This was the notification Feature');

    });

To this:

    return admin.messaging().sendToDevice(token_id, payload).then(response => {

        console.log('This was the notification Feature');
        return null;   // add this line

    });

The then callback just needs to return a value.

However, eslint may then complain about nested then() in your code, which is also an anti-pattern. Your code should really be structured more like this:

const deviceToken = admin.database().ref(`/ServiceProvider/${user_id}/device_token`).once('value');
return deviceToken.then(result => {
    // redacted stuff...
    return admin.messaging().sendToDevice(token_id, payload);
}).then(() => {
    console.log('This was the notification Feature');
});

Note that each then chains off of each other, rather than being nested inside each other.

Doug Stevenson
  • 297,357
  • 32
  • 422
  • 441
  • Hey, Thanks man I have solved the issue, but fcm deliver the message if my device is connected to internet at the very moment, If I turned on internet after 30 sec, notification is lost somewhere. – Ikram Khan Niazi Nov 18 '18 at 05:56
  • 1
    @Doug Stevenson, I applaud all your contributions to Firebase. But how come there are so many places on the Firebase Cloud functions web site, where they never show this part of the code, correctly. I had even written to the Feedback link to correct the code(add extra **return** where necessary), exactly like, what you have shown here, but of no avail. – Yo Apps Jul 07 '19 at 20:07
  • @DougStevenson why did you say "The then callback just needs to return a value" but your second example the `then` only has the log function, with no return value? – Lance Samaria Oct 19 '21 at 15:48
  • @YoApps this Google lab shows a similar example of what your asking, it's pretty good too https://developers.google.com/web/ilt/pwa/working-with-promises#catch – Lance Samaria Oct 19 '21 at 15:52
3

Change this:

    return admin.messaging().sendToDevice(token_id, payload).then(response => {

    console.log('This was the notification Feature');

  });

into this:

    return admin.messaging().sendToDevice(token_id, payload).then(response=>{
      console.log('This was the notification Feature');
      return true;
    },err=>
    {
      throw err;
    });

As the error says when using then you need to return a value.

Peter Haddad
  • 78,874
  • 25
  • 140
  • 134
  • 2
    It's not necessary to capture and re-throw the error. A rejected promise will propagate up the promise chain all the way to Cloud Functions. – Doug Stevenson Nov 17 '18 at 19:06
  • "Returning" `undefined` as the value from the `then` callback is just as fine (if you don't need another value). – Bergi Nov 17 '18 at 19:06
  • Hey, Thanks man I have solved the issue, but fcm deliver the message if my device is connected to internet at the very moment, If I turned on internet after 30 sec, notification is lost somewhere. – Ikram Khan Niazi Nov 18 '18 at 05:56
0

That's jslinting telling you that each .then must include a return value. In other words, avoid promise anti pattern.

You might find async functions easier to wrap your head around. Note that you'll need to run the Node 8 runtime for async support though...

Ronnie Royston
  • 16,778
  • 6
  • 77
  • 91