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

Production having warnings about firestore backend #681

Open
jkomoros opened this issue Dec 6, 2023 · 7 comments
Open

Production having warnings about firestore backend #681

jkomoros opened this issue Dec 6, 2023 · 7 comments
Labels
bug Something isn't working

Comments

@jkomoros
Copy link
Owner

jkomoros commented Dec 6, 2023

[2023-12-06T04:01:51.682Z]  @firebase/firestore: Firestore (10.6.0): Could not reach Cloud Firestore backend. Backend didn't respond within 10 seconds.
This typically indicates that your device does not have a healthy Internet connection at the moment. The client will operate in offline mode until it is able to successfully connect to the backend.

Shows up on any load after the first from a fresh profile.

I thought this originally was caused by the storage outage, but it persists after that.

Cards created in this state do show up on the server viewer, although the warning spooks me (and also incidentally means that suggestions don't work because the cards never get fully loaded)

@jkomoros jkomoros added the bug Something isn't working label Dec 6, 2023
@jkomoros
Copy link
Owner Author

jkomoros commented Dec 6, 2023

See also #665.

@jkomoros
Copy link
Owner Author

jkomoros commented Dec 6, 2023

I wonder if I crossed some loading threshold on number of cards? Things actually appear to be loaded.

Tried updating firebase, and trying without longpolling and without cache (but that triggers the original issue in #665).

Adding cards, they appear to actually be loaded and edits going to the server, so it appears the warning is erroneous? Still it's pretty spooky, it seems to warn that there could be a data loss error if you don't sync...

jkomoros added a commit that referenced this issue Dec 6, 2023
Part of #681.

Doesn't appear to fix anything.
@jkomoros
Copy link
Owner Author

jkomoros commented Dec 6, 2023

firebase/firebase-tools#6162 I thought was causing this at first (because I was also running into this after a failed deploy, but that's since been cleaned up.)

@jkomoros
Copy link
Owner Author

It seems like there are two issues: 1) an erroneous error showing up because it takes a long time to validate the large corpus (this is a guess) and 2) a "cards not fully loaded" error that happens due to a timing error.

There's been a long-time bug where we signal expectUnpublishedCards once, but then there are three distinct unpublished cards types, and some code paths that return an empty one will bail early before reporting that they did indeed get downloaded. This is a race that is now resolving differently due to the size of the payload (I guess)

Anywhere that takes unpublished : boolean should take type CardPublishedType = 'published' | 'unpublished_all', 'unpublished_editor', and 'unpublished_author'. And then disconnectLiveUnpublishedCardsForUser should unregister if there's a need to. And updateCards() shoud take CardPublishedtype, and send an empty object if necessary... which the reducer should note there are no items and then unset the flag of that type.

jkomoros added a commit that referenced this issue Dec 15, 2023
Currently doesn't do too much, but does technically fix a weird edge case in signaling.

Part of #681.
jkomoros added a commit that referenced this issue Dec 15, 2023
Instead of a single unpublisheCardLoaded, we instead have a different loading flag for each type of
loading, that is set and then cleared.

This SHOULD fix the race condition (or at least address it) in #681 where a long-loading thing was now
changing how a race resolved.
@jkomoros
Copy link
Owner Author

I think the recent timing change broke the "make a new working notes card" flow.

Now when you hit the working notes button or use the shortcut, it creates a card, then navigates to it outside a collection, and then doesn't open it for editing. I wonder if it's the logic for selectCardsLoaded, which I noted might be overly simplified in the change.

This also triggers the #684 more often (or it's possible that was created by that issue)

@jkomoros
Copy link
Owner Author

jkomoros commented Dec 17, 2023

Bisect confirms it was ba3f7c8 that broke it

removeCards and actuallyRemoveCards looks very supicious (but hasn't been touched in a long time)

The problem is in EXPECT_NEW_CARD reducer, which sets publishedCardsLoaded or unpublishedCardsLoaded directly and thus doesn't get a warning about the thing.

Perhaps logic somewher in waitForCardToExist is too quick? And audit timing of navigateToNewCard being fired

jkomoros added a commit that referenced this issue Dec 17, 2023
…ags.

This does NOT fix the issue in #681, but does definitely fix up some broken logic and is likely at least a subsantial part of the fix.

Part of #681.
jkomoros added a commit that referenced this issue Dec 17, 2023
…t collection.

ef476f5 was a partial fix to ba3f7c8, but it was partially broken because it modified the supposedly immutable loadingCardFetchTypes of the previous state... oops! This mean that the cardsLoaded selector saw object equality and assumed no change, when in reality it had changed, so dataIsFullyLoaded erroneously still returned true.

Part of #681.
@jkomoros
Copy link
Owner Author

A bug: if you create a new card before the final computed state of the last edited card settles (e.g. before all new fingerprints are calculated), it seems to navigate to the new card outside the collection (and not open for editing). Some kind of race condition that didn't used to exist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant