Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require await on all Promises #1993

Closed
TheModMaker opened this issue Jun 12, 2019 · 1 comment
Closed

Require await on all Promises #1993

TheModMaker opened this issue Jun 12, 2019 · 1 comment
Labels
priority: P3 Useful but not urgent type: code health A code health issue

Comments

@TheModMaker
Copy link
Contributor

Have you read the FAQ and checked for duplicate open issues?
Yes

Is your feature request related to a problem? Please describe.
With the transition to async/await, there are a lot of cases where we just ignore the Promise that is returned. This is fine for async work, but means that errors can be easily ignored. We still did this before with ignored Promise chains, but is probably more prevalent now with async/await.

Describe the solution you'd like
We should have something that checks that we "use" the returned Promise. We might be able to write a compiler plugin to do this. Since the compiler knows the types of fields and return values, we might be able to track the Promise variables and verify they are either in an "await" or returned.

There are many cases where we want to "start" a job but not wait for it to be completed. For example, in StreamingEngine we have several cases where we don't want to block the caller but the operation is asynchronous. We could create some wrapper that can be used to monitor these async events and call an onError handler if something goes wrong. This will also make the code clearer when we want to start async work vs just forgetting to use await.

There is also the case where we use event listeners that are async. Event listeners are synchronous and even ignore thrown exceptions. Since we use our EventManager, we can use that to wrap any event callbacks to handle this. We can catch any thrown exceptions and also monitor any Promises that are returned.

Describe alternatives you've considered

Additional context

@TheModMaker TheModMaker added the type: code health A code health issue label Jun 12, 2019
@TheModMaker TheModMaker added this to the v2.6 milestone Jun 12, 2019
@joeyparrish joeyparrish modified the milestones: v2.6, Backlog Feb 12, 2020
@TheModMaker TheModMaker added the priority: P3 Useful but not urgent label Sep 29, 2021
@avelad
Copy link
Collaborator

avelad commented Apr 10, 2024

I have reviewed the code and async/await is always used in promises except in cast_proxy and cast_receiver, but this is scheduled to be removed in #4214

@avelad avelad closed this as completed Apr 10, 2024
@avelad avelad removed this from the Backlog milestone Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: P3 Useful but not urgent type: code health A code health issue
Projects
None yet
Development

No branches or pull requests

3 participants