136

I'm using Express.js in my code with Node.js v7.3. In this I've created a User Router which forwards the requests to my User Controller.

I'm using async/await inside the User Controller to do asynchronous calls. The problem is that IntelliJ gives me a warning saying that

Promise returned from login() is ignored.

The thing is I'm not even returning anything from the login() method.

Here's the code -

UserRouter.js

router.post('/login', function (req, res, next) {
    userController.login(req, res); // I get the warning here
});

UserController.js

exports.login = async function (req, res) {
    try {
        const verifiedUser = await someFunction(req.body.access_code);
        let user = await User.findOrCreateUser(verifiedUser);
        res.status(200).send(user);
    }
    catch (err) {
        res.status(400).send({success: false, error: err});
    }
};

If I write the same login method using native promises only then I don't get this warning. Am I understanding something wrong here or is IntelliJ at fault?

EDIT -

Thanks to @Stephen, I understand that an async function returns a promise but wouldn't it be better if Intellij identifies that nothing is being returned from the async function and doesn't show that warning because when I chain a .then() after the login() function, it provides an undefined object into the then result. It means if we don't return something from the async function explicitly then undefined is returned?

Jyotman Singh
  • 10,792
  • 8
  • 39
  • 55
  • 1
    While the promise resolves to `undefined`, the IDE is warning you that you're ignoring the fact that it resolves or rejects at all and when. It would be nice if you could mark functions as "the promise from this may be safely ignored" so you don't have to mark it ignored at each call site. – David Harkness Sep 08 '21 at 17:45

9 Answers9

116

You should use the "void" operator.

From MDN: void is for "evaluating expressions that produce a value into places where an expression that evaluates to undefined is desired."

router.post('/login', function (req, res, next) {
    void userController.login(req, res); // Warning will not be shown now
});
JaviCasa
  • 698
  • 8
  • 17
gzen
  • 1,206
  • 1
  • 8
  • 6
  • 16
    This is the best answer in my opinion. This is exactly what `void` was designed for. [From MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/void#description): void is for "evaluating expressions that produce a value into places where an expression that evaluates to undefined is desired." – ChrisCrossCrash Apr 13 '21 at 17:31
  • 2
    If I do this, SonarLint gets mad at me instead with a `Critical javascript:S3735` ""void" should not be used" with description "The void operator evaluates its argument and unconditionally returns undefined. It can be useful in pre-ECMAScript 5 environments, where undefined could be reassigned, but generally, its use makes code harder to understand." – Bryan K Dec 03 '21 at 21:42
  • 1
    This answer would be better with some explanation. – isherwood Jun 27 '22 at 13:21
  • @ChrisCrossCrash So using `void` is the right approach regarding to @BryanK comment? – Jakub Słowikowski Aug 16 '22 at 13:30
  • Omg! This is the best answer, yet not understandable at first glance. The explanation must be paced before code snippet because I only see the code and the comment in it, then I saw a void statement then I guessed that it should be the thing to try, I tried it and see it worked then I see the explanation :D – semihlt Aug 18 '22 at 09:34
83

The userController.login() function returns a promise, but you're not doing anything with the result from the promise by utilizing its then() function.

For example:

userController.login(req, res).then(() => {
    // Do something after login is successful.
});

or in the ES2017 syntax:

await userController.login(req, res);

If you don't actually want to do anything there, I guess you can just ignore the warning. The warning is mostly there because not using the then() function on a promise is usually a code smell.

Robba
  • 7,684
  • 12
  • 48
  • 76
  • 2
    So, is it a good approach to ignore a promise function return value if I dont need in the series of execution of my code ? – NIKHIL C M Oct 16 '18 at 12:15
  • 1
    If you really don't care if the function succeeds or fails, and don't want to do anything with the returned value once the promise has resolved, you can safely omit the `.then()` call. – Robba Oct 16 '18 at 15:02
  • 26
    I usually do `let ignore = asyncFunction();` This makes intelliJ stop complaining (plus it won't complain about unused variable if it's called ignore). Additionally it is showing that I am explicitly ignoring the promise and haven't just forgotten it. – Bob Vale Feb 27 '19 at 12:08
  • Making the caller function async and adding the await keyword made the inspection disappear. – Markus Zeller Aug 17 '20 at 09:21
  • 3
    @MarkusZeller this is functionally different from the original question though. In the original question the promise was (deliberately) ignored. When using async/await, you no longer ignore the promise, but any statements after the await will in fact wait for the promise completion. – Robba Aug 17 '20 at 13:28
  • 1
    @Robba As there are none as last expression, that will be no problem. But, thank you for that detail I did not think about. – Markus Zeller Aug 17 '20 at 14:03
  • 1
    @BobVale now idea freaks out because you have an unused constant ;-) – Marv Feb 03 '22 at 09:38
  • 1
    @Marv It shouldn't because the variable is called ignore. Unless they've changed the logic to no longer bypass that inspection for a variable called ignore. Fairly certain it won't complain if you use _ as the variable name – Bob Vale Feb 03 '22 at 13:36
  • @BobVale It seems to throw the warning if you assign the returned promise to a variable named `_`, but not one named `ignore`. Function definition parameters named either `ignore` or `_` will not cause a warning. – Matthew Jensen Jul 18 '22 at 22:55
73

The thing is I'm not even returning anything from the login() method.

A function declared "async" returns a Promise by definition. See for example https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function

However the IDEA warning is only an inspection. You can press "alt-enter, right" on the warning and change the inspection level to make the warning go away. The inspection is in the "JavaScript -> Probable bugs" category and is named "Result of method call returning a promise is ignored".

eekboom
  • 5,551
  • 1
  • 30
  • 39
  • 9
    In WebStrom, disable [Preferences] - [Editor] - [Inspection] - [JavaScript] - [Probable bugs] - [Result of method call returning a promise is ignored]. – MJ Studio Feb 16 '20 at 09:31
  • 4
    In Idea, disable [Preferences] - [Editor] - [Inspection] - [JavaScript and TypeScript] - [Async code and promises] - [Result of method call returning a promise is ignored]. – Emmanuel Osimosu Jul 13 '20 at 16:58
  • 2
    It would be nice if there were a JetBrains equivalent of `// eslint-disable-line ...`. Is there one? – jacobq May 11 '22 at 22:30
34

if you are really manic as me and the then() is not required but you need the warning to go away, a possible solution is:

functionWithAsync.error(console.error);

// or use a logger
functionWithAsync.error(log.error);
fingerprints
  • 2,751
  • 1
  • 25
  • 45
  • 17
    Or `functionWithAsync.catch(console.error);` with other versions of Promise (like the one in ionic 5.2). – Anthony O. Mar 04 '19 at 23:51
  • 1
    You don't explain why, but this is a really good solution because it handles any internal rejections that might otherwise get gobbled up and not thrown. If someone calls promise.reject and you don't await or handle the rejected promise, it's gone. Often in my code I actually want to throw this error in order to stop execution. – light24bulbs May 22 '21 at 20:03
  • 2
    @AnthonyO. Would you or someone else please elaborate on your specification of `.catch()` over `.error()`? Looking around it seems .catch() is now the norm. – Rob Sep 23 '21 at 06:10
26

another way to get rid of the warning is defining an empty then():

userController.login(req, res); // <- Get the warning here

userController.login(req, res).then(); // <- No warning

Alejandro Silva
  • 8,808
  • 1
  • 35
  • 29
  • 1
    Thank you! This is perfect for Ionic templates so that you don't have to ignore the warning with messy comments or disable in the IDE. – Citizen Jul 17 '19 at 17:53
9

If you simply want to shut this warning off for any of JetBrains products. Go to

Preferences > Inspections > JavaScript and TypeScript | Async code and promises | Result of method call returning a promise is ignored and turn the setting off.

Nate Bunney
  • 2,418
  • 2
  • 22
  • 31
7

I'm using try{} catch(e){} in NodeJs and found that simply adding Error() to the end of the function fixed the warning.

Full code:-

someArray.forEach(async (arrayValue) => {
    try {
        const prodData = await myAsyncFunc(arrayValue);
    } catch(e) {
        console.error(`Error: ${e}`);
    }
}, Error());
Joe Keene
  • 2,175
  • 21
  • 27
1

In Intellij Ide

1. Hover over the yellow warning

enter image description here

2. Click on the More actions

enter image description here

3. Expand the options of first line In this case Add .then arrow sign

enter image description here

4. Then Click on the Edit inspection profile setting

enter image description here

5. Unchecked Result of method call returning a and then ok

Aamir Kalimi
  • 1,821
  • 2
  • 15
  • 19
0

functionWithAsync.catch();

In Angular it can be:

private async someMethod() {

 await this.asyncMethod.catch();

}
Tatyana Molchanova
  • 1,305
  • 5
  • 12
  • 23