Made sure that more of the async usages are awaited #4574
Conversation
| await sendInfo(socket); | ||
| await server.sendMaintenanceList(socket); | ||
| await sendNotificationList(socket); | ||
| await sendProxyList(socket); | ||
| await sendDockerHostList(socket); | ||
| await sendAPIKeyList(socket); | ||
| await sendRemoteBrowserList(socket); |
There was a problem hiding this comment.
In my opinion, using await or not is depending on the design. The need of blocking and non-blocking.
Here, most of them are actually intended not to use await, because they are independent. And for MariaDB, they can query at the same time without doing one by one.
There was a problem hiding this comment.
Agreed. But also, the final user experience will still depend on many factors. For example, if we are IO-bound on the server-side, there is no benefit to blasting off the responses all at once, and awaiting makes the application behavior more predictable. Having to sleep(500) seems crude in comparison.
There was a problem hiding this comment.
Maybe something like Promise.all() would be a good compromise?
There was a problem hiding this comment.
I think in this case Promise.allSettled() might be better, but yes (am going to propose another PR incl. remmoving the sleep)
I was afraid of running into weird unreproducible race conditions, but in this case this is likely ok ^^
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma
Tick the checkbox if you understand [x]:
Description
This PR makes more of our async usages awaited and fixes the annotations for those async functions where we are missing them.
The former was mostly found via WebStorm's inellisense, while the latter were found by looking at every async file.
Type of change
Please delete any options that are not relevant.
Checklist
Screenshots (if any)
Please do not use any external image service. Instead, just paste in or drag and drop the image here, and it will be uploaded automatically.