59

I'm developing Server with Firebase.

I had copied Google Developer's Video on Youtube.

It works well, but on log there is an error:

Function returned undefined, expected Promise or value

It says function returned undefined, but I make function return a promise `set``

How can I solve this?

function sanitize(s) {
    var sanitizedText = s;
    console.log('sanitize params: ', sanitizedText);
    sanitizedText = sanitizedText.replace(/\bstupid\b/ig, "wonderful");
    return sanitizedText;
}
exports.sanitizePost = functions.database
    .ref('/posts/{pushId}')
    .onWrite(event => {
        const post = event.data.val();
        if (post.sanitized) return;

        console.log('Sanitizing new post', event.params.pushId);
        console.log(post);
        post.sanitized = true;
        post.title = sanitize(post.title);
        post.body = sanitize(post.body);
        return event.data.ref.set(post); 
    })

I'm beginner of Firebase, Nodejs.

Frank van Puffelen
  • 565,676
  • 79
  • 828
  • 807
Josh
  • 1,042
  • 1
  • 9
  • 17
  • Try `if (post.sanitized) return true;` – Frank van Puffelen Nov 06 '17 at 07:02
  • 2
    @FrankvanPuffelen: This post has been getting a lot of views. In a comment from AaronJo on my answer below , he shares that Firebase Support indicated the change requiring a Promise or scalar return value was intentional and will remain. Can you share any insights regarding why the change was made? What are the benefits? When a non-Promise value is returned, does the value have any meaning? – Bob Snyder Nov 23 '17 at 17:28
  • 23
    When you explicitly return a value, it is clear that the function is done. When you explicitly return a promise, it's clear that the function needs to remain active until the promise is resolved/rejected. When you don't return a value, it is not clear what state the function is in. – Frank van Puffelen Nov 23 '17 at 19:03
  • 1
    @FrankvanPuffelen any chance https://firebase.google.com/docs/functions/terminate-functions could be updated? It directs people to just "return;" still, which confused me for a bit. – Pat Jun 11 '18 at 23:13
  • I don't see what's wrong on that page, the only "return" instruction is "Terminate a synchronous function with a `return;` statement.", which is for *synchronous* functions, where it is correct afaik.This question was about an asynchronous function, which had a missing return value. – Frank van Puffelen Jun 11 '18 at 23:24

4 Answers4

61

As Frank indicates in his comment on your post, the return statement that is producing the warning is this one:

if (post.sanitized) return;

The warning can be silenced by returning a dummy value (e.g. null, false, 0). The value is not used.

Earlier versions of Cloud Functions did not complain when a function exited using a return statement with no value. That explains why you see return; in the video you linked and in the documentation. The comment on the question by Firebaser Frank van Pufeelen, explains why the change was made.

The simplest way to eliminate the warning is to add a return value, as Frank suggested:

if (post.sanitized) return 0;

Another option is to change the trigger from onWrite() to onCreate(). Then the function will not be invoked when the post is sanitized and the check that produces the warning is not needed:

exports.sanitizePost = functions.database
    .ref('/test/{pushId}')
    .onCreate(event => {  // <= changed from onWrite()
        const post = event.data.val();
        //if (post.sanitized) return; // <= no longer needed

        console.log('Sanitizing new post', event.params.pushId);
        console.log(post);
        //post.sanitized = true; // <= not needed when trigger is onCreate()
        post.title = sanitize(post.title);
        post.body = sanitize(post.body);
        return event.data.ref.set(post);
    });
Bob Snyder
  • 37,759
  • 6
  • 111
  • 158
  • Will this check be removed any time soon? Otherwise we'd have to change a lot of code to silence those error warnings in the logs. – railgun Nov 18 '17 at 09:37
  • @AaronJo: Contact Firebase Support: https://firebase.google.com/support/contact/bugs-features/ – Bob Snyder Nov 18 '17 at 13:16
  • From what I was told by the firebase support it sounds like things are going to remain this way. – railgun Nov 20 '17 at 15:39
  • @AaronJo: Thanks for sharing the response from Firebase Support. Not clear to me what the benefit of requiring a return value is. – Bob Snyder Nov 20 '17 at 15:49
  • It's not clear to me either. I had to add `return 0;` to the end of a function just to get rid of this log error. I'd really like to know what the benefit is for having to add this (to me) extraneous line of code. Maybe @frank-van-puffelen or someone else from the Firebase team could explain the logic of it? – thoragio Nov 23 '17 at 00:06
  • @thoragio: See FvP's new comment on the question. – Bob Snyder Nov 23 '17 at 19:39
  • @BobSnyder Awesome. Thank you both. – thoragio Nov 25 '17 at 13:40
2

I was getting this same error for attempting to read a document using .get() which returns a promise.

I found out in the official Firebase YouTube tutorial that to resolve this error, I needed to return the promise line of code. Check minute 4:18 in the tutorial linked video [https://youtu.be/d9GrysWH1Lc]

Also, as a side note, what lead me to the tutorial solution is when I noticed in the function logs did actually log valid values but only after the function is closed, even though the error says the result was undefined.

Samer
  • 3,848
  • 4
  • 25
  • 24
2

Adding to what @bob-snyder said, your problem is that your returning undefined under a condition.

if (post.sanitized) return;

My suggestion is to use a single exit point, which is a common tip when programming. Article.

Example

// code...
const SUCCESS_CODE = 0;

exports.sanitizePost = functions.database
    .ref('/posts/{pushId}')
    .onWrite(event => {
        const post = event.data.val();

        let response = Promise.resolve(SUCCESS_CODE);

        if (!post.sanitized) {
            console.log('Sanitizing new post', event.params.pushId);
            console.log(post);
            post.sanitized = true;
            post.title = sanitize(post.title);
            post.body = sanitize(post.body);
            response = event.data.ref.set(post); 
        }

        return response;
    })
ajorquera
  • 1,297
  • 22
  • 29
2

I noticed that when the trigger handler is an async function, the warning isn't raised even if we don't have an explicit return statement.

When the trigger handler is a sync function, we'd need an explicit return statement to silence the warning, like like it was suggested in the accepted answer.

That is, if we rewrote

exports.updatePost = functions.firestore
.document("posts/{postId}")
.onUpdate(async (snap, context) => { /* async handler*/
  ...
});

to

exports.updatePost = functions.firestore
.document("posts/{postId}")
.onUpdate((snap, context) => { /* sync handler*/
  ...
});

We'd need to do return 0; or similar inside the sync handler.

In any case, for async, it's probably a good idea to explicitly return a promise on the async handler, as advised on the official docs:

Resolve functions that perform asynchronous processing (also known as "background functions") by returning a JavaScript promise.

kip2
  • 6,473
  • 4
  • 55
  • 72