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

Fix freeze when signing out on web #573

Merged
merged 2 commits into from
Apr 6, 2023
Merged

Conversation

nilsreichardt
Copy link
Member

Through Git binary search, I found out that the issue #497 was caused by #469. I don't why the web app crashes when the AppUser is TypeOfUser.unknown. We are never using the TypeOfUser.unknown state. However, using TypeOfUser.student works and I think should be okay as a fallback value, because typeOfUser should never be null.

Screen.Recording.2023-04-06.at.00.48.29.mov

Closes #497

@docs-page
Copy link

docs-page bot commented Apr 5, 2023

To view this pull requests documentation preview, visit the following URL:

docs.page/sharezoneapp/sharezone-app~573

Documentation is deployed and generated using docs.page.

@github-actions
Copy link

github-actions bot commented Apr 5, 2023

Visit the preview URL for this PR (updated for commit e9f1e76):

https://sharezone-test--pr573-fix-freeze-when-sign-m37zyxf0.web.app

(expires Thu, 13 Apr 2023 11:34:14 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 4cb3ae61e1e018abfd9841fd3239f5b49ccc034b

@nilsreichardt nilsreichardt added platform: web feature: navigation Navigation inside the app (e.g. switching to a different screen). labels Apr 6, 2023
@Jonas-Sander
Copy link
Collaborator

If it never should be null then we should actually investigate why this is happening or not?
Could you open an issue for this?

@nilsreichardt
Copy link
Member Author

I opened #608

@nilsreichardt nilsreichardt added this pull request to the merge queue Apr 6, 2023
@nilsreichardt nilsreichardt removed this pull request from the merge queue due to the queue being cleared Apr 6, 2023
@nilsreichardt nilsreichardt added this pull request to the merge queue Apr 6, 2023
Merged via the queue into main with commit f3de604 Apr 6, 2023
@nilsreichardt nilsreichardt deleted the fix-freeze-when-signing-out branch April 6, 2023 23:46
github-merge-queue bot pushed a commit that referenced this pull request Jul 6, 2023
## Summary

This PR fixes:

* #608 
* #654 
* #497 (real fix)

It's required to this all in one pull request because otherwise it would
introduce a new bug.

## What was the issue?

I started to debug on this problem because of #654. I found out it was
because `TypeOfUser.student` was used as a backup when it's null (see
#573). However, just reverting it to `TypeOfUser.unknown` was not option
because this would introduce #497 again.

Therefore, I wondered, why `typeOfUser` can be null. The reason for this
was the last time online reporter. Because `.set()` has been used for
Firestore, Firestore instantly was setting it null (I think `null` is
used for `Firestore.currentTimestamp()` when the client is offline or
the server hasn't responded yet). Using `.update()` fixed the issue that
the user document only contained `lastOnline`.

But using `update()` in the last online reporter always caused #497.
Even using `TypeOfUser.student` or `TypeOfUser.unknown` couldn't be as a
workaround. After hours of debugging I found that when signing our the
following line was causing the issue:


https://github.com/SharezoneApp/sharezone-app/blob/47e4ccb0993f01b5c9b8c83f31f7ec4c4f833c77/app/lib/util/api/homework_api.dart#L107

The issue is that we listened to `_homeworkSubjectStream` and when an
error occurred, we the `.addError` method of `_homeworkSubjectStream `
used. This ended up into an infinity loop which resulted in the freeze
(#497),

This was by far one of my hardest bugs... I needed ~5,5h to find the
solution and it was extremely hard to debug.

Fixes #608
Fixes #654
Fixes #497

---------

Co-authored-by: Jonas Sander <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: navigation Navigation inside the app (e.g. switching to a different screen). platform: web
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App freezes when signing out
2 participants