I have a large project that has now been migrated over from javascript to typescript and we're very pleased with the results. Bugs have fallen dramatically with the improved compiler support in combination with rigorously written code.
Ours is a server-side product that runs under node (ts-node, actually). By its nature, it is highly asynchronous, so the move to Typescript had the major benefit of providing us access to async/await. It has been working really well, and dramatically reduced our code size and complexity.
The one problem it has introduced is that we occasionally miss putting an "await" keyword in front of calls to methods returning promises (i.e., other async functions) when inside an async function. This "send and forget" is something that one occasionally might want to use, but for the most part, this creates a lot of chaos in our design pattern, as in most cases, the call is to something that will complete asynchronously, but most of the time we need to wait for that to complete. (That method cannot be synchronous because it depends on external services that are asynchronous.) It is not uncommon in our case to have 5 or 10 await statements in a single method.
A missing "await" keyword can be hard to find -- especially when the method being called doesn't return anything that is needed by the caller. (If it returns something, type-checking will usually catch the mismatch between the type of the Promise
Ideally, we could have a "nowait" keyword that would go where one would otherwise put an "await" to indicate to the Typescript compiler that the design intent is to explicitly NOT WAIT for the called method to be completing. The tsc compiler could have a new flag that controls whether to create a warning if a method call returns a Promise that is missing either "await" or "nowait". The "nowait" keyword would have no effect whatsoever on the compiled code.
I _strongly_ agree with the problem this issue is trying to address. My colleagues and I continue to hit this all the time. (I'd estimate at least once a week.) And it frequently masks bugs.
It goes without saying that this only happens on void
methods. But a lot of times, those void
methods are significant, e.g. validateFoo
, checkAccess
, async test
s, etc.
I imagine the specific solution proposed in this issue (of a nowait
keyword) will be challenging since async/await
is an ES7 feature, not a TypeScript one. But here are other related issues:
https://github.com/Microsoft/TypeScript/issues/10381
https://github.com/palantir/tslint/issues/892
I'd be a fan of having this check built into the compiler rather than the linter, since it's truly a _correctness_ issue, not a style issue.
Thanks to everyone for the consideration!
I have recently started migrating a huge project from promises to async/await. having this feature can really ease such migrations. It's very hard to detect dangling promises until something goes terribly wrong with hours of trying to reproduce the issue only to find a function call was missing await
😱
This would be highly valuable but the problem with a noawait
keyword is that it introduces new expression level syntax.
However, we can avoid this problem by taking the idea and introducing a flag, --requireAwait
(:bike:🏠), which would trigger an error if a top level expression of type Promise<T>
within an async
method is not awaited _and_ its value is not part of an expression or an assignment.
To suppress the error, the language could require a type assertion, which would otherwise be meaningless
declare function f(): Promise<void>;
// --requireAwait
async function g() {
f(); // Error
f() as {}; // OK
await f(); // OK
const p = f(); // OK
p; // Error
p as {}; // OK
Promise.all([f(), p]); // Error because of .all
await Promise.all([f(), p]); // OK
return f(); // OK
}
There are some issues with this idea though.
const p = f()
is currently valid but likely indicates an error so encouraging it could be badPromise<void>
to return a value is generally preferable(but sometimes not possible or desirable).@aluanhaddad I like that idea.
Since typescript is going to introduce decorators like @@deprecated
how about @@requireAwait
? Might even be able to provide an implementation that actually catches these in run time. No more flags too :grin:
It will be opt-in.
Edit: or maybe opt-out with a flag: --requireAwait
and @@ignoreAwait
@alitaheri that would be _much_ cleaner. Did you mean @@deprecated
?
yes, I had forgotten that it was a bit different. Thanks for the reminder 😅 😅
@aluanhaddad: I love your proposal. My colleague @ryanwe has had a similar thought: this issue can be caught when a caller doesn't make use of a returned Promise.
I don't feel like the issues you list are a big deal. To me, it's a major step forward for the compiler to help guard against the 90% bug case, even if it doesn't address the last 10%. (The linter can help w/ that last 10%, e.g. const p = f()
and you don't make use of that p
.)
@alitaheri: I might not understand your proposal. Where would I put this @@requireAwait
decorator? On every single async
method I have? (I see your edit now for flipping that to @@ignoreAwait
👍 )
There's no need for nowait
- the void
operator already exists for a consuming an expression while producing no side effects (e.g. you could just write void doSomethingAsync();
)
@RyanCavanaugh I think you are missing a key point. The problem is not about whether the promise gets executed. The problem is that the writer forgets to put the "await" in front of a call to an async method/function and gets a behavior that he/she is not expecting. So I believe this is a compiler warning/error issue only -- not a functional one. I'd be fine if this was "opt-in" in the sense that one would have to choose to turn on a compiler option to get warnings about calls to methods returning promises inside async functions that are unused. If using this compiler option, the 'nowait' keyword could be used if the writer has the (rare) need to invoke the method but not wait for the result.
@kduffie I think what @RyanCavanaugh is saying is that a flag is sufficient and that no new syntax is required. That is a great thing. I don't think he is dismissing the issue as it is now marked as in discussion.
The funny thing my initial type level syntax suggestion, was going to involve asserting that the type of the expression was void
async function f() {
returnsPromise() as void;
}
which is a type error.
It never occurred to me to use
async function f() {
void returnsPromise();
}
Which is far more elegant and is a JavaScript construct 😅
Ah. I see. Sorry for the confusion. He's saying that "void" would tell the compiler that you understood that promise returned is intentionally unused. With the new compiler flag present, one would get an error if there is neither an await nor a "void". Got it. Just fine with me.
@kduffie I believe so. I think that would be a practical solution, but we will see.
Chatted with @rbuckton about this and his take was that we could enforce this rule in async
functions only with a flag without much trouble. Certainly the value proposition is clear - this stuff sounds like a nightmare to debug.
The "in async
" would be needed because innocuous code like this (in a non-async function):
getSomePromise().then(() => whatver());
would be incorrectly flagged as an error because then
itself returns a Promise.
And yes as usual @aluanhaddad has cleared up my terse comments with precision and insight
Approved as the default behavior (i.e. no flag):
async
function for the expression of an expression statement to be exactly a function call or method call whose return type is Promise
Workarounds would be e.g. void f();
or <any>o.m();
for intentionally-discarded Promises.
If anyone thinks there are good examples of functions that return Promises where this would be burdensome, please speak up now with specifics!
Edit: Improved clarity of rule
can believe its for spproved for asyncs/promises only
I think this is a fairly common pattern for firing off a bunch of async work at once and then awaiting in one spot.
declare function fetchUser(): Promise<any>;
declare function fetchAccount(): Promise<any>;
async function loadStuff() {
const userPromise = fetchUser();
const accountPromise = fetchAccount();
const [user, account] = await Promise.all([userPromise, accountPromise]);
//do stuff
}
the example @jwbay mentioned would still be allowed. the ones that would be flagged as errors are expression statements. e.g.
async function loadStuff() {
fetchUser(); // error, did you forget await?
const accountPromise = fetchAccount(); // OK
}
I tried the new no-floating-promises rule in tslint 4.4 and it works great!
EXCEPT that I'm using VisualStudio code and its tslint integration won't show this error because this rule requires type checking and it appears that that isn't supported.
Anyone know if there are plans to fix VSCode so that it can handle type checking rules?
@kduffie we're working on an extensibility model that will allow TSLint to report errors in editors
tslint already shows errors in the Problems window in vscode (using the tslint extension) but I'm now seeing the following -- which I assume is a limitation of the tslint extension.
Is that what you mean when you say, "... an extensibility model"?
vscode-tslint: 'no-floating-promises requires type checking' while validating: /Users/kduffie/git/kai/ts_modules/task-helper.ts
stacktrace: Error: no-floating-promises requires type checking
at Rule.TypedRule.apply (/Users/kduffie/git/kai/node_modules/tslint/lib/language/rule/typedRule.js:34:15)
at Linter.applyRule (/Users/kduffie/git/kai/node_modules/tslint/lib/linter.js:138:33)
at Linter.lint (/Users/kduffie/git/kai/node_modules/tslint/lib/linter.js:104:41)
at doValidate (/Users/kduffie/.vscode/extensions/eg2.tslint-0.8.1/server/server.js:369:20)
at validateTextDocument (/Users/kduffie/.vscode/extensions/eg2.tslint-0.8.1/server/server.js:285:27)
at documents.forEach.err (/Users/kduffie/.vscode/extensions/eg2.tslint-0.8.1/server/server.js:274:13)
at Array.forEach (native)
at validateAllTextDocuments (/Users/kduffie/.vscode/extensions/eg2.tslint-0.8.1/server/server.js:272:15)
at connection.onDidChangeWatchedFiles (/Users/kduffie/.vscode/extensions/eg2.tslint-0.8.1/server/server.js:523:9)
I asked for this ages ago and was told to use a linter for it instead...
At the end of my issue I said "linting works for us", but that was a lie.
How can I use a linter for it when the linter cannot tell that the return type of an arbitrary functions is a promise or not, whereas the typescript compiler can?
I ran into _exactly_ this issue today. I have an async function that validates user input, and throws to a try/catch
block lower in the stack if any problems are found.
Missing await
had the _really_ nice side-effect of bypassing this frontline check and assuming all user input was good, resulting in attempted DB inserts, privileged sessions, and a whole host of other things that I'd consider 'class A' bugs, as well as dropping this cryptic hint to the console:
(node:1151) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): [object Array]
api_1 | (node:1151) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
Chaos from just one innocent missing await
. And over an hour going line by line in a 10K LOC+ project to track it down.
@RyanCavanaugh, I see you submitted a PR for this 6 months ago (thanks!). How long do these kinds of features typically take to get merged/released? Today has proven for me just how useful this check is!
We've addressed this shortcoming by using tsliint and including
"no-floating-promises": true in tslint.json. tslint will complain if you
don't handle a promise. And if you make the explicit choice not to wait for
the promise (using await), you can put "void" in front of the expression
that returns the promise. Works well for us. Of course, I'd prefer having a
"nowait" keyword, but haven't had any big problems since using this tslint
approach.
On Tue, Oct 31, 2017 at 4:46 PM, Lee Benson notifications@github.com
wrote:
I ran into exactly this issue today. I have an async function that
validates user input, and throws to a try/catch block lower in the stack
if any problems are found.Missing await had the really nice side-effect of bypassing this
frontline check and assuming all user input was good, resulting in
attempted DB inserts, privileged sessions, and a whole host of other things
that I'd consider 'class A' bugs, as well as dropping this cryptic hint to
the console:(node:1151) UnhandledPromiseRejectionWarning: Unhandled promise rejection
(rejection id: 1): [object Array]
api_1 | (node:1151) [DEP0018] DeprecationWarning: Unhandled promise
rejections are deprecated. In the future, promise rejections that are not
handled will terminate the Node.js process with a non-zero exit code.Chaos from just one innocent missing await. And over an hour going line
by line in a 10K LOC+ project to track it down.@RyanCavanaugh https://github.com/ryancavanaugh, I see you submitted a
PR for this 6 months ago (thanks!). How long do these kinds of features
typically take to get merged/released? Today has proven for me just how
useful this check is!—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/Microsoft/TypeScript/issues/13376#issuecomment-340939490,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAd7Qx9hWIv65vHq0X4uqeJNJgFaLzMKks5sx7FXgaJpZM4Le8NG
.
Thanks for the tip @kduffie, I'll use that as a meantime solution. Would be good to see this merged in TS proper ultimately, though; I feel it's a fundamental part of the type system rather than something that should be delegated/demoted to linting.
@kduffie, it doesn't seem that no-floating-promises
is actually firing for my use-case. This is the code:
export const getStatesQuery = {
args: {
country: {
type: GraphQLString,
},
},
resolve: resolver<IStates>(async (_root: null, args: IStateArgs) => {
const e = new FormError();
const data = trim<IStateArgs>(args);
validate<IStateArgs>(data, e, ["country"]); // requires await -- yet no error
// Get states on country
return {
states: getStates(data.country),
};
}),
type: graphqlState,
};
In this case, validate<T>
is an async function that returns Promise<FormError>
. I can't see why the rule isn't picking this up?
Edit:
Actually, ignore the above. It's just a limitation of VSCode handling linting rules that require type info.
Just a guess, but I wonder whether tslint is confused by the async inline
function declaration. If you refactor that as a separate named method,
does it catch it then?
On Wed, Nov 1, 2017 at 2:34 AM, Lee Benson notifications@github.com wrote:
@kduffie https://github.com/kduffie, it doesn't seem that
no-floating-promises is actually firing for my use-case. This is the code:export const getStatesQuery = {
args: {
country: {
type: GraphQLString,
},
},
resolve: resolver(async (_root: null, args: IStateArgs) => {
const e = new FormError();
const data = trim(args); validate<IStateArgs>(data, e, ["country"]); // requires await -- no error // Get states on country return { states: getStates(data.country), };
}),
type: graphqlState,
};In this case, validate
is an async function that returns
Promise. I can't see why the rule isn't picking this up? —
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/Microsoft/TypeScript/issues/13376#issuecomment-341052703,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAd7Qxmu87_i0OBttkegKdncN9B6E2SPks5syDshgaJpZM4Le8NG
.
@kduffie - I made the following edit above (apologies- you probably didn't see this by email):
Actually, ignore the above. It's just a limitation of VSCode handling linting rules that require type info.
@leebenson I did, indeed, miss your edit. Thanks for clarifying. But now I'm curious about your statement (in case it may affect me in future). You say, "... limitation of VSCode handling linting rules...) Am I correct that you're talking about types that are declared in other files in the project? If so, we use tslint across a whole project at a time, and it works well for us. The only problem is that in VSCode, you have to have a separate task to run tslint, since Code can't run it constantly. In our case, we always run tslint manually before every checkin.
@kduffie - if you're already running tslint
as a task and/or manual commit step and you're okay with this extra step, then this issue won't affect you.
In my case, I'm using the TSLint plugin to check code _as I type_, which for the most part, works perfectly. But it misses the await
check:
The reason is because no-floating-promises requires type info, which the TSLint for VSCode plugin readme recommends solving as a VSCode task:
I'm new to VSCode (I switched from Atom solely for the Typescript support), so I'm not sure if there are any obvious workarounds or whether there's a setting I can change somewhere to make it 'type aware' in the editor. In any case, I'm running tslint
as a pre-commit step too... it'd just be nice to get that instant feedback as I'm tapping out code, as well.
@leebenson You're correct. No way yet to get tslint to use type information from other files in the project while you type. Sad, but the Code team has really been doing fantastic stuff and I'm sure they're going to get to this eventually. We're VERY pleased with typescript and Code for developing it. Huge productivity gains.
Yup, same here @kduffie. Like I said, only early days for me with Code, but Typescript support is first class and the speed improvements over Atom were a nice surprise. I'm still finding my way around, but I can't imagine switching back!
In light of the discussion about errors vs warnings we should instead move this to tslint.
If tslint is still not capable of looking beyond the file it’s in to determine whether a function is a async call or not... then maybe this is a job for ts
@jpike88 TSLint is capable of determine if a function returns promise or not
Looks like I confused it with simply just being a broken function.
@Hoishin actually, on further investigation, I have to jump through a bunch of weird hoops just to get no-floating-promises
working in VSCode.
I should have read further up in this chain to realise that, but it's not exactly intuitive.
https://github.com/palantir/tslint/issues/3399
@mhegazy @adidahiya I think there is a real disconnect here regarding VSCode, TS and TSLint, as its clear that TSLint offers fantastic enhancements to the coding experience, and it should be allowed to operate with minimal friction.
I understand why they removed type checking from TSLint, it's so there aren't effectively two TSServer processes running side by side. But in doing that, some TSLint flags which are incredibly useful become a pain to utilise.
IMO VSCode should meet TSLint in the middle and expose type information via some sort of side channel, so that TSLint can use all of its flags smoothly. Is that feasible?
I think this issue should be closed
Closed why? This problem still exists and is very hard.
It can be an optional feature.
tsconfig.json
{
"compilerOptions":{
"needNowait": true
async dbFetch():Promise<Row>{
return new Promise(...){
...
}
}
var myRow = dbFetch(); //must raise an error
var dbTask = nowait dbFetch(); // ok nowait (nowait emits nothing),
var row = dbTask; //must raise an error dbTask is Promise<Row>
var row = await dbTask; //ok
var dbTask:Promise<row> = dbFetch(); // ok because dbTask is a Promise and that is explicit
I cannot imagine this being in scope of TypeScript core. As I mentioned above, TSLint (or ESLint) can ensure you’re using await
on promise-returning function calling.
Ensuring await
is not enough. We need a way to say 'here is correct to not await'.
// tslint-disable-next-line no-floating-promises
const foo = promiseFunction()
@emilioplatzer As the one who originally opened this issue, I can tell you that the answer is simple. Where you would normally put "await", add "void". For example, if you have a function that returns a promise called "foobar", then
await foobar(); // This will wait for foobar to finish
void foobar(); // This will not necessarily wait for foobar to finish before proceeding
Adding "void" in the second case does not change the behavior. It just signals to tslint that you have decided not to use the promise that is returned, and so it will not report an error. Our process now uses tslint (with "no-floating-promises") and everything is working great for us. (I'd prefer the compiler itself would alert us to this, but that would just be a bonus.)
I'd prefer the compiler itself would alert us to this, but that would just be a bonus.
Same here. Missed await can potentially lead to serious system issues and it is not always easy to guess missed await was a reason of which. So I'd like compiler be able to catch the issue rather than a linter which is harder in proper configuring. I'd also prefer the await check would be included in the grouping strict
compilation option.
I cannot imagine this being in scope of TypeScript core. As I mentioned above, TSLint (or ESLint) can ensure you’re using await on promise-returning function calling.
I personally disagree. Ensuring a Promise is explicitly awaited or discarded is very much a core concern, especially when the distinction is subtle and can often be overlooked.
This isn't the purview of a linter- it's a fundamental control flow issue.
Where you would normally put "await", add "void"
void
is perfectly fine in lieu of noawait
, but there should be a flag to enforce handling a Promise in tsconfig.json (possibly even enabled by default).
The point is, it's very easy right now to fire a Promise and never do anything with it, which may later throw an exception that isn't properly handled, and bring down a running server. I've seen this _so_ many times, across a number of projects with different teams.
What's the point in having a compiler that can tell us a variable hasn't been used (a non-exception generating event) if it allows us to fire off a Promise without being explicit that we're okay not catching possible errors that may occur at some random point in the future?
Same here - I saw so many code, where simply is no awareness of a missing await. The Typescript compiler is the perfect place to show this as an error. It has the most information about async / Promise functions in the project or external libraries.
I would go even further would activate this check in strict mode. Using void or no-await or !await would be all fine.
I'd be really curious to hear from the downvoters on this issue because it is definitely a big source of errors. 2:1 👍 to 👎 is an outlier ratio and it'd be great to understand their thoughts.
@RyanCavanaugh when I first filed this, I'd put this as "red hot" because the errors resulting from this can be terrible to debug. At this point, however, I'd put this as "like to have" because we've baked tslint into our overall process. It would just be preferable to catch these right in the editor, rather than waiting for project-wide lint.
It would just be preferable to catch these right in the editor
Does ms-vscode.vscode-typescript-tslint-plugin help?
At this point, however, I'd put this as "like to have" because we've baked tslint into our overall process.
While it's nice that you've found a solution with tslint, I think the fundamental question is: is this really the job of a linter / external lib to catch?
And if it _is_ the job of a linter, why do we have noUnusedLocals
and other flags in TS proper? Why not move _all_ code checks outside of type errors to tslint?
IMO, this feels more like a fundamental control flow issue at least on par with many of the flags already settable in tsconfig.
@RyanCavanaugh I thought TypeScript's view was to not introduce additional keywords wherever possible. I'm of the opinion that forgetting to add await
is something that should be handled by a linter, not the compiler.
@RyanCavanaugh
It's nice to have this kind of checking, yes, I completely agree with it, but I don't know if this benefits the users more than creating messes (more options, more keywords, etc).
TypeScript has Promise
type already. Unintended promise-returning function handling will eventually result in type errors somewhere if you properly uses TypeScript.
@RyanCavanaugh I thought TypeScript's view was to not introduce additional keywords wherever possible. I'm of the opinion that forgetting to add
await
is something that should be handled by a linter, not the compiler.
This wouldn’t introduce another keyword. void
is sufficient. It would simply check that a) the Promise was being used/awaited or b) was prepended with void.
TypeScript has Promise type already. Unintended promise-returning function handling will eventually result in type errors somewhere if you properly uses TypeScript.
I think you're missing the subtleties of this issue.
Right now, something like this is valid:
async function willEventuallyThrow(): Promise<void> {
setTimeout(() => throw Error("this will throw after 10 seconds"), 10000)
}
//... inside the body of another function...
try {
willEventuallyThrow(); // <-- returns a void Promise, so not 'consumable'; but should succeed
someCodeThatExpectsTheAboveToResolve();
} catch (e) {
// ... error will never be handled, since it's not scoped to this block via `await`
}
^^ The above scenario is _very_ common. TS won't complain that a Promise has been fired that's not awaited or returned. Forgetting await
here completely changes the control flow... the two functions can execute arbitrarily and independently, and willEventuallyThrow()
's exception will bubble up the stack since there's no catch block boundary.
As many others here have suggested, this is a common problem that is easy to fall into because there are no TS warnings and it's easy to miss. You only realise when it blows up at runtime.
The tslinter does not fix the common problem. It only detects not assigned promsies.
For example here:
]
the y
variable is now a Promise. If I have the choise to say the compiler (or the linter) that allways check if I wrote await
or noawait
in each function that returns a promise I was happy because I do not do that silly error.
To put noawait
each time that is needed is not painfull because are very few cases when I need to do that. In my work not need await
is rare. When I do not write await
is in the most cases a mistake.
Of course if I write the types of each variable (y
in this case) I detect the error. But Typescript is about to strict check without the need of writing types in all places. Type inference is the best gift!
I don't myself think const someName = somePromiseReturningThing();
is a big hazard given no-unused-vars
+ standard type-checking should be able to detect this, either you haven't used the variable or it won't be assignable to how you wanted to use it anyway (console.log
is a pretty special case in that it takes any
).
Having said that the expression statement case is a large hazard, I think it's probably worth solving if there's a lone expression that happens to be a Promise. One option that involves no new syntax would just be to ignore expressions that are prefixed by void
.
e.g.:
async function main() {
// Not an error as the expression is "used"
void fireAndForgetSomething();
// Error as Promise is never used
fireAndForgetSomething();
}
main();
Perhaps it's a bit strange but it would at least be entirely solvable in a type-aware linter without introducing new syntax.
Approved as the default behavior (i.e. no flag):
- It is an error in an
async
function for the _expression_ of an _expression statement_ to be exactly a _function call_ or _method call_ whose return type isPromise
Workarounds would be e.g.
void f();
or<any>o.m();
for intentionally-discarded Promises.If anyone thinks there are good examples of functions that return Promises where this would be burdensome, please speak up now with specifics!
_Edit:_ Improved clarity of rule
What is meant by "approved as the default behavior"? If that is the case, are there any plans to implement this? I would be fine with having it as an opt-in feature, but I do agree with the others in this thread who have supported the notion that forgetting to await a promise is worth a TS warning as it affects control-flow in significant and often destructive ways.
(i read through this issue and it seems like the discussion has moved on from the nowait proposal. i'd recommend a title change since there are somewhat recent comments re. nowait)
and -- i didn't notice a comment above that explicitly calls out this extremely difficult to debug situation and i have no workaround in ts as it stands today:
async function someAsyncCode(): Promise<number> {
throw new Error('ffffffffffffffffffffff');
}
// we want to call something async and handle the errors
async function consumer(): Promise<null | number> {
try {
// BUT: this line forgets the await which significantly changes the behavior
return someAsyncCode();
}
catch (err) {
console.log('handled error', err);
return null;
}
}
// as a developer you'd glance at consumer() and mistakingly assume it handles all errors
// when in fact it doesn't (common dev mistake).
consumer().then(result => console.log('done!', result));
// the result is an UnhandledPromiseRejectionWarning in node and developer confusion.
Right now there is no error in ts because technically consumer() is returning a Promisereturn someAsyncCode()
and return await someAsyncCode()
have two very different meanings.
I have no solution to this problem because sometimes you want to return a Promise from an async function and sometimes you don't but it's definitely a problem.
TS really needs to warn on missing awaits. It doesn't need a new keyword to do it.
I would be happy if the compiler warned about return promiseFn()
in async
functions. It's never what I want to do. I have of course returned a promise directly from a function, but never from an async
one except by accident.
Returning a Promise from within an async function is perfectly valid, since they’ll just be chained. I don’t think that’s the issue here.
The bigger issue is that ignoring Promises should be explicit, to avoid unintended side-effects such as an async func throwing an exception sometime later.
The void
keyword is ideal for this, IMO.
By the way, for those who use tslint and depend on no-floating-promises to
detect missing "await", I strongly recommend that you also add
"no-promise-as-boolean". Without that, you may run into problems (like I
did recently) from missing awaits when you call async methods inside
if/while/for statements.
https://palantir.github.io/tslint/rules/no-promise-as-boolean/
On Wed, Sep 9, 2020 at 11:54 PM Lee Benson notifications@github.com wrote:
Returning a Promise from within an async function is perfectly valid,
since they’ll just be chained. I don’t think that’s the issue here.The bigger issue is that ignoring Promises is should be explicit, to avoid
unintended side-effects such as an async func throwing an exception
sometime later.The void keyword is ideal for this, IMO.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/microsoft/TypeScript/issues/13376#issuecomment-690029919,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AADXWQZ5MWTW7JYB57FBGN3SFBZ3HANCNFSM4C33YNDA
.
@leebenson Are you missing the issue when the return is in a try
block and your catch
or finally
blocks do something important?
My point was that returning the result of a Promise inside an async func has the same practical effect as returning the Promise itself (assuming you don't care about the immediate value, which you obviously do if you're wrapping in a try/catch block). Having to do return await somePromise()
to appease the linter for the plain case strikes me as redundant, but probably makes sense if its immediate scope is a try
block.
Most helpful comment
There's no need for
nowait
- thevoid
operator already exists for a consuming an expression while producing no side effects (e.g. you could just writevoid doSomethingAsync();
)