Skip to content

Conversation

sefk
Copy link
Contributor

@sefk sefk commented Apr 20, 2018

This example has been broken since this pull request on Feb 2018:
https://github.com/firebase/functions-samples/pull/306/files. Fixes "result not
defined" and "metadata not defined" errors but the overall control flow was
also pretty broken.

sefk added 2 commits April 19, 2018 23:11
This example has been broken since this pull request on Feb 2018:
https://github.com/firebase/functions-samples/pull/306/files. Fixes "result not
defined" and "metadata not defined" errors but the overall control flow was
also pretty broken.
@nicolasgarnier
Copy link
Contributor

@sefk Thanks for the fix! I'll change this a bit since we need to follow a strict styling on this repo now and nested Promises are not allowed (which caused the original error to begin with).

@samtstern any idea why the travis build passed? I do see the ESLint errors in there

@sefk
Copy link
Contributor Author

sefk commented Apr 20, 2018

Thanks for taking a look, @nicolasgarnier. Changed to unnest promises. Not sure if the state passing in line 57-58 is the best way to do that, feels kinda clunky.

Also glad that you're following up on why our automated tests didn't catch. Maybe other similar problems were introduced in that same pull request?

@nicolasgarnier
Copy link
Contributor

Thanks for this @sefk ! Just tweaked it a bit (we need to return the admin.database()... promise to make sure the async jobs are chained) and also we enforce returning in every promise to avoid the mistake where developer forget to return Promises which result un functions terminating too early.

@nicolasgarnier nicolasgarnier merged commit e973fe6 into firebase:master Apr 20, 2018
@sefk
Copy link
Contributor Author

sefk commented Apr 23, 2018

Appreciate the cleanups! Thanks.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants