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

Internal sync loop is recursive w/ promises #734

Closed
ffrostfall opened this issue Jul 14, 2023 · 3 comments · Fixed by #739
Closed

Internal sync loop is recursive w/ promises #734

ffrostfall opened this issue Jul 14, 2023 · 3 comments · Fixed by #739

Comments

@ffrostfall
Copy link

The internal syncing loop is implemented recursively, using promises as an abstraction. This is bad because it might be hiding bugs that cause the syncing loop to die without raising any trace, and it generally is not a good way to do syncing.

A better alternative could be a simple non-recursive while loop. This is better for readability, and more straightforward.

Any other alternatives would be appreciated!

@boatbomber
Copy link
Member

boatbomber commented Jul 15, 2023

How would it die without a trace? The purpose of the recursive nature is so that we keep returning a promise that we :catch (line 150 of ServeSession) to ensure we don't miss an error.

@boatbomber
Copy link
Member

I'll write a non-recursive version and see if there's any difference. Worst case, the code got a little simpler and easier to understand. Best case, a bunch of weird bugs disappear.

@boatbomber
Copy link
Member

I've actually found a likely culprit of the silent death! We use hanging promises in ApiContext to make things just never resolve. If that happens when it shouldn't, there's nothing to :catch and we have silent death. Don't know how to reproduce that, but yeah there's probably some edge case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants