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

Possible issue in the new ShapeManager code. #1283

Closed
davidmartos96 opened this issue May 20, 2024 · 3 comments · Fixed by #1247
Closed

Possible issue in the new ShapeManager code. #1283

davidmartos96 opened this issue May 20, 2024 · 3 comments · Fixed by #1247
Assignees

Comments

@davidmartos96
Copy link
Contributor

cc @msfstef

While porting the new ShapeManager refactor to Dart we found an issue that we are unsure if it could cause unexpected problems.

this.promises[fullKey].resolve()

The following line is accessing the promise resolver unconditionally, but it can be undefined. It currently throws, but the exception is not bubbled up because of the Promise.allSettled in the AsyncEventEmitter.

In fact, one of the tests reaches this situation, the one named: a subscription that failed to apply because of FK constraint triggers GC

Copy link

linear bot commented May 20, 2024

@msfstef
Copy link
Contributor

msfstef commented May 21, 2024

Thanks for reporting this! Indeed we had noticed that the AsyncEventEmitter was suppressing errors and we had a PR open to fix that (see #1247).

While working on that PR I managed to reproduce this error, you can see a description of what was happening here: #1247 (comment)

The above PR should also resolve this issue once merged.

@davidmartos96
Copy link
Contributor Author

@msfstef I see, thanks!
We encountered this hidden error really by chance, as our Dart implementation wasn't doing the equivalent of Promise.waitSettled.

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