Audit Promise return values

[dpogue]

There are too many places where we have very inconsistent return types involving promises. Particularly cases where functions throw, return a resolving promise, and also return a rejecting promise. Also a lot of places where functions return promises that resolve with different value types in different paths.

HookRunner is one spot I've seen that's particularly bad for this, but I've seen it in a few other spots too.

My general recommendation:

• Any function that returns a promise should never throw. It should reject the promise.
• Any function that returns a promise should document the resolving value type of that promise, and ensure that only that value type is ever resolved.

[raphinesse]

Any function that returns a promise should never throw. It should reject the promise.

To clarify: It's OK if throw statements are used in async functions or inside Promise callbacks, because in these cases the Exceptions will be converted into Promise rejections.

[PasunuriSrinidhi]

To audit a function's return values

1. Identify the expected return values.
2. Review the function code.
3. Test the function with different input values.
4. Check error handling.
5. Consider side effects.