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

Followup to infinite loop in eventsAfter (#2544) #2553

Closed
taoeffect opened this issue Jan 27, 2025 · 5 comments · Fixed by #2555
Closed

Followup to infinite loop in eventsAfter (#2544) #2553

taoeffect opened this issue Jan 27, 2025 · 5 comments · Fixed by #2555
Assignees
Labels
App:Frontend Kind:Bug Kind:Core Anything that changes or affects the fundamental core data structures & design of the application. Priority:High

Comments

@taoeffect
Copy link
Member

taoeffect commented Jan 27, 2025

Problem

Followup to #2544 (we didn't test it).

So I tested this by using our GIG group DB and a DM with myself.

  • I made a copy of the DB, sent myself a few DMs and switched to #general, then shut down the server, replaced the newer DB with the older copy, and re-ran GI_PERSIST=sqlite grunt dev
  • Then I visited my DM with myself again to load it. I was greeted with the alert* and clicked OK.
  • The chat fixed itself (I think; I might've had to first switch to #general and back, please test this), however the red banner remained visible when it should be removed since the problem is fixed
  • Furthermore, another problem appeared, which was the backend being hammered with these requests:

Image

*When I tried this a second time, I oddly enough wasn't greeted with the alert, but did see the constant streamEntriesAfter on the backend... odd.

Solution

Test this scenario thoroughly locally and fix all remaining bugs, specifically:

  • The red critical banner should be dismissed if the resync was a success
  • The infinite loop should be fixed for good

EDIT: it seems the infinite loop was fixed; issue is now about:

  • Log BOTH the GI_VERSION and the GI_GIT_VERSION = git describe --dirty in the service worker when it starts up, so that we can be 100% sure that we're running the latest version.
  • Immediately dismiss the danger banner after the user clicks OK to resync the contract.
  • Log the URL for eventsAfter when an exception is thrown (in production, the parameters are not logged)
@taoeffect taoeffect added App:Frontend Kind:Bug Kind:Core Anything that changes or affects the fundamental core data structures & design of the application. Priority:High labels Jan 27, 2025
@corrideat
Copy link
Member

Are you sure you didn't accidentally switch to a different branch? The alert not showing and the stream after requests would seem to suggest this as a possibility. So far, I've not been able to reproduce this issue. Are you still able to reproduce?

The red critical banner should be dismissed if the resync was a success

Maybe, but I'm not sure how this can be done? The banner is 'stateless', meaning that after it's generated, there's no state about what it's about. I think we could clear it, but we risk clearing a different banner.

@taoeffect
Copy link
Member Author

The alert not showing and the stream after requests would seem to suggest this as a possibility.

I very clearly wrote: I was greeted with the alert

Are you sure you didn't accidentally switch to a different branch?

I am positive. This was reproduced on current master.

I think we could clear it, but we risk clearing a different banner.

Yes 👍

@corrideat
Copy link
Member

You said:

When I tried this a second time, I oddly enough wasn't greeted with the alert, but did see the constant streamEntriesAfter on the backend... odd.

@taoeffect
Copy link
Member Author

Correct, it's weird behavior that sometimes showed the alert and sometimes didn't. All on master.

@taoeffect
Copy link
Member Author

taoeffect commented Jan 28, 2025

Update: I tried again and wasn't able to reproduce the infinite loop issue the second time around. So what might have happened is that the service worker didn't get updated.

To wit, let's make this issue now just about:

  • Log BOTH the GI_VERSION and the GI_GIT_VERSION = git describe --dirty in the service worker when it starts up, so that we can be 100% sure that we're running the latest version.
  • Immediately dismiss the danger banner after the user clicks OK to resync the contract.
  • Log the URL for eventsAfter when an exception is thrown (in production, the parameters are not logged)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App:Frontend Kind:Bug Kind:Core Anything that changes or affects the fundamental core data structures & design of the application. Priority:High
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants