112

I'm doing some basic asynchronous operations using async/await in TypeScript but TSLint is throwing mysterious error messages for these two functions below. Has anyone encountered these errors before? On the error output the governing rule is not mentioned, so I don't understand what's causing these. Any ideas would be greatly appreciated.

The main request:

import * as rp from 'request-promise'

export function getRequest(address: rp.Options): rp.RequestPromise {
  return rp(address)
}

Exported async function:

export async function getStatus(message: Message) {
  try {
    const res = await getRequest(address)
    if (res.ready) {
      message.reply('...')
    } else {
      message.reply('...')
    }
  } catch (err) {
    message.reply(err)
  }
}

This gets: Promises must be handled appropriatelyand await of non-Promise for line #3.

The simple function that uses this export is:

client.on('message', message => {
  if (message.content === 'green') {
    getStatus(message)
  }
})

This also gets Promises must be handled appropriately.

Additional information:

Even though the error message doesn't mention it, this seems to be the governing rule for Promises must be handled appropriately: https://palantir.github.io/tslint/rules/no-floating-promises/

And this Issue mentions await of non-Promise: https://github.com/palantir/tslint/issues/2661

T04435
  • 12,507
  • 5
  • 54
  • 54
cinnaroll45
  • 2,661
  • 7
  • 24
  • 41
  • 3
    Can you also post the getRequest function in the question, thanks. – MrGoofus May 15 '17 at 13:00
  • Great point, I've added it. – cinnaroll45 May 15 '17 at 13:07
  • Just guessing here, but this could be that tslint doesnt recognize that the rp function returns a promise. You could try setting a type for it, export function getRequest(address: rp.Options): Promise { ... Let me know if this works so I don't spend more time checking this :) – MrGoofus May 15 '17 at 13:13
  • Returning a Promise errors out on the IDE with not being compatible to what's actually being returned by `rp`. So I used this: `export function getRequest(address: rp.Options): rp.RequestPromise { return rp(address) }` This satisfies the IDE but I'm still getting exactly the same errors in the initial post. – cinnaroll45 May 15 '17 at 13:19

6 Answers6

156

That's a crappy error message. A better one might be,

every expression of type Promise must end with a call to .catch or a call to .then with a rejection handler (source).

So, for example, if you do

PromiseFunction()
  .catch(err => handle(err))
  .then(() => console.log('this will succeed'))

then you will still have a tslint problem, because the type of .then(...) is a promise, and it has to end with a catch. The fix would be appending a .catch clause, for example,

PromiseFunction()
  .catch(err => handle(err))
  .then(() => console.log('this will succeed'))
  .catch(() => 'obligatory catch')

or just disabling tslint for that line via:

PromiseFunction()
  .catch(err => handle(err))
  // tslint:disable-next-line:no-unsafe-any
  .then(() => console.log('this will succeed'))

Alternatively, you could reverse the order of the .then and .catch statements. However, that stops the .then from executing if an error does occur, which you presumably want if you encountered this problem.

Elliott Beach
  • 10,459
  • 9
  • 28
  • 41
  • 38
    Hi Elliot - I wrote the `no-floating-promises` rule and I agree the error message isn't great. If you want to send a PR with a better description I'm all for it! – Josh Jun 22 '18 at 20:04
  • I thought having a floating promise is fine if I'm not making parent function async and I really want the promise to be floating because it does not impact the rest of functional logic. What am I missing here? – anandbibek May 28 '23 at 14:25
  • @anandbibek I'm saying this is a bug in the linter. Does that help? – Elliott Beach May 28 '23 at 20:05
  • @ElliottBeach I'd say yes. Because this is first time I am seeing SonarLint is messing with my functional logic without any clear way to mitigate the warning. – anandbibek May 30 '23 at 12:43
110

Sometimes you might want to call the promise, but you don't need to do anything with the response. A route change or something else.

so instead of:

promiseFunction().then().catch()
try/catch async/await

you can do:

void promiseFunction();

As per the comments on the correct use of void please read: IgnoreVoid

Use at your own convenience :)

T04435
  • 12,507
  • 5
  • 54
  • 54
  • 8
    nice this works, it'd be handy to see a reference to the `void` syntax – Damian Green Mar 12 '19 at 18:11
  • This works great, but for the life of me I cannot find any reference to this in any documentation anywhere. – ach Aug 05 '19 at 17:10
  • 1
    Apparently this is the [JavaScript "void" operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/void). – ach Aug 05 '19 at 17:24
  • Putting a void in front of the called function worked. – Sarthak Srivastav Jan 22 '20 at 12:14
  • Don't use this. You just obfuscated it. It's still a floating promise. And it's a non-common use of this. – Domske Jan 27 '20 at 14:06
  • Hi @Dominik could you please expand on your comment, thanks. – T04435 Jan 28 '20 at 06:53
  • 4
    @T044350 It's about "Avoid void". Void is an operator that evaluates the given expression and returns undefined. It's often used in minified / uglified code, because `void 0` is shorter than `undefined`. And it is used as "pure" undefined. In clean code it can be confusing and it entices to disguise problems. There are many articles in the web. (just google it. "eslint no-void") The words here are limited. :D Finally it's up to you (the developer) how to write code and for what context. (professional clean code or amateur. The freedom in JavaScript is a blessing and a curse at the same time.) – Domske Jan 28 '20 at 14:36
  • @Dominik and what's the issue with a floating promise? What if I just want to kick it off and don't care about the result? – El Mac Feb 16 '21 at 08:13
  • 1
    [Here are the docs](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/docs/rules/no-floating-promises.md) for the rule in question, including a section about the use of `void`. You can use inline comments to disable the rule on a case by case basis, but I personally don't use `void` for anything else, so I find `/* checked, can't reject */ void promiseFunc();` perfectly legible. – Coderer Sep 14 '21 at 15:40
  • The `void` one helped, and there can be so many reasons when we want to run an async function and don't want it to be blocking. Don't see why using it should be avoided. – Munim Dec 27 '22 at 07:31
7

I have got the same exception when i have created firebase-function using firebase-tool

const ref = admin.database().ref("path/to/database/object");

ref.once("value").catch(error =>{  // line 22
    response.send( error() );
}).then( snapshot =>{
    response.send( snapshot.val );
})

This code doesn not compiled and return

ERROR: /src/index.ts[22, 5]: Promises must be handled appropriately

I have changed the places of catch and then.

ref.once(...).then(...).catch(...)

This code is work, i am sorry but I don't have any explanation

So much amazing when app return the some error without catch block even according firebase doc not mentioned that catch is required.

Vahe Gharibyan
  • 5,277
  • 4
  • 36
  • 47
5

Your getStatus function is defined to return a promise:

// All functions marked as async returns a promise:
async function getStatus(message: Message) {/* ... */}

But you called getStatus without calling it's then:

getStatus(message)

Therefore the compiler thinks you've forgotten to handle your async code. All you need to do is call .then():

getStatus(message).then(() => console.log('done'));
slebetman
  • 109,858
  • 19
  • 140
  • 171
  • You're right but this line `getStatus(message).then(() => console.log('done'))` still gets `Promises must be handled appropriately`. – cinnaroll45 May 15 '17 at 14:27
  • @cinnaroll45 Did you try handling the `.catch()` as well? `getStatus().then().catch()` – slebetman May 16 '17 at 00:03
  • Yeah, tried bot `.catch()` and a `try/catch` for the invocation of `getStatus()` still the same error as my first comment. – cinnaroll45 May 16 '17 at 12:54
  • 1
    @cinnaroll45 were you able to solve this issue? I am also having same issue and unable to solve that – Prafful Garg Oct 04 '17 at 08:33
  • No luck @PraffulGarg, ended up switching `no-floating-promises` off. No matter how I handled the errors, I was unable to satisfy that rule. – cinnaroll45 Oct 04 '17 at 15:26
  • 4
    I downvoted because this is just wrong, tslint is looking for a catch or .then with rejection handler. – Elliott Beach Feb 04 '18 at 02:16
2

If you are writing a void async function with typescript-eslint, you will be getting a complaint if you write async functions without await, it's easy to forget especially for the last line of the function.

const sendSyncPlayCommandToWatchers = async (): Promise<void> => {
  ...
  someAsyncFunction()
}

You will get

error Promises must be handled appropriately @typescript-eslint/no-floating-promises

You need to add the await keyword to the last line (or make sure you handle the promise with try/catch as mentioned in previous replies

const sendSyncPlayCommandToWatchers = async () => {
  ...
  await someAsyncFunction()
}

I think this will also help getting a better stack trace when debugging.

Cyril Duchon-Doris
  • 12,964
  • 9
  • 77
  • 164
1

I think this problem is fixable by awaiting the getStatus function, since its an async function. The message is saying it clearly, but the line number causes confusion. To be honest it also took me some time.

You can solve this lint error by this change in code:

client.on('message', message => {
 if (message.content === 'green') {
   await getStatus(message)
}});

In my opinion it's not a good idea to switching these specific errors off. They are useful because in the other way you would not run the code async.

Crix
  • 86
  • 3
  • 1
    This will change the behavior of the code. Before adding the `await`, `getStatus` would not hold up the thread. The author may not want to delay subsequent execution. This could needlessly hold up an app. – jmealy Aug 07 '19 at 20:25