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: listen to user doc changes instead of fetching doc to check if exists #226

Closed

Conversation

sripwoud
Copy link
Member

@sripwoud sripwoud commented Oct 30, 2023


name: Pull Request
about: Open a PR for p0tion

Description

See https://discord.com/channels/943612659163602974/1166371125450768495/1167435859755278357
It looks like in case of a lag between the auth command and the contribute command, the user document may not yet exist in firebase.
As a solution we can listen for document changes and wait for the document to exists().

Fixes #220

How Has This Been Tested?

  • See unit test for waitForUserDocumentToExist

My unit test is a bit artificial so I am 100% my fix works as expected, I'd like to run additional manual e2e testing, but I am not sure how...

Checklist:

  • My code follows the style guidelines of this project
    This PR deliberately doesn't follow style guidelines to avoid big diff, I suggest merging chore: format, lint #225 first then rebase
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I reviewed the code of conduct and contributors' guide

…ng doc to check if exists

There can be some lag after running the auth command before the user can be found in firebase. In
which case trying to fetch the user will fail with `FirebaseError: Unable to retrieve the
authenticated user.`. Instead we can listen and wait for user doc changes.

privacy-scaling-explorations#220
@sripwoud sripwoud changed the title fix #220: listen to user doc changes instead of fetching doc to check if exists fix: listen to user doc changes instead of fetching doc to check if exists Oct 30, 2023
@ctrlc03 ctrlc03 self-requested a review November 2, 2023 10:21
@@ -123,6 +124,26 @@ export const getDocumentById = async (
return getDoc(docRef)
}

export const waitForUserDocumentToExist = (firestoreDatabase: Firestore, collection: string, documentId: string) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while this would be a great solution, the code might hang forever if a user tried to auth and they did not pass the sybil checks. As discussed, a fetch and retry mechanism might work better for now until we find a more elegant solution

@@ -150,6 +158,29 @@ describe("Database", () => {
})
})

describe("waitForUserDocumentToExist", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great stuff adding a test for it

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 this pull request may close these issues.

There might be a lag in between auth and contribute resulting in an error
2 participants