Error when permissions to a document are removed from a collection with openDBChannel running

[RichardCr]

Hi there,

Not sure if this is an edgecase or if I am doing something wrong... I am getting an error and sync issues when permissions to a document are removed that is inside a collection with openDBChannel running.

I have setup role based permissions to docs (I call boards). This was done by having a roles object on each board that has this format roles: { uid: "owner", uid: "editor"} as suggested here.

For a user to get the boards they have permission to see I use:

const where = [
  ['roles.' + user.uid, 'in', ['owner', 'editor', 'viewer']]
]
dispatch('boards/openDBChannel', {clauses: { where }}, { root: true }).catch(console.error)

This works perfectly in some regards:

• New boards appear
• Boards that you are invited to appear (i.e. a field added to roles with your uid),
• Boards that are deleted disappear,

However as mentioned, boards where your permissions are removed (field removed from roles with your uid) cause the following error in console.

'compareObjectProps' can only compare objects index.esm.js?34ed:113 [vuex-easy-firestore] Error! Something went wrong during the PATCH mutation: The document it's trying to patch does not exist.

There is then a little bit of weirdness with firebase and vuex being out of sync, that is mostly fixed with a refresh.

Any idea how to handle this case? Thanks for any help!

[louisameline]

Hello there! Thank you for the report. Do you use the latest version of the library, and do you catch errors properly? Your snippet looks like it's using an old version.

[RichardCr]

Hi, Thanks for the reply :) The code was written a long time ago, but the version is up to date I think "vuex-easy-firestore": "^1.35.3" As for error catching properly, I think so? I'm still pretty new to this.. Any suggestions?

[louisameline]

Take a look at the doc, you have to catch errors of the streaming promise returned by openDBChannel. If you do it there, you might prevent further errors.

[RichardCr]

When I try the new syntax

          dispatch('boards/openDBChannel', {clauses: { where }}, { root: true })
            .then(({streaming}) => {
              streaming()
                .then(() => {
                  console.log("streaming started")
                  // this gets resolved when you close the channel yourself
                })
                .catch(error => {
                  console.log(error)
                })
            })
            .catch(error => {console.log(error)})

I get a console error TypeError: streaming is not a function any ideas ? Much thanks for your help so far!

[louisameline]

You're welcome! Is streaming undefined then? Have you updated the version of the library in your package.json and run npm install (or yarn's equivalent)?

[RichardCr]

Yeah... npm list in my working directory gives: vuex-easy-firestore@1.35.6 Confusing :/ The error comes from the bottom .catch(error => {console.log(error)}) in the above code.. is there any import or anything that is needed?

[RichardCr]

console.log(streaming) gives

Promise {<pending>, isFulfilled: false, resolve: ƒ, reject: ƒ, …}
resolve: ƒ ()
reject: ƒ ()
isFulfilled: false
isRejected: false
isPending: true
__proto__: Promise
[[PromiseStatus]]: "pending"
[[PromiseValue]]: undefined

So it isn't undefined

[louisameline]

Alright, it's normal that the "bottom catch" will catch an error which exists at the moment the channel gets opened. The "streaming catch" will catch errors that happen after the channel has started working. So, if you change rights after the channel has successfully started, it's in the streaming error handler that you'll know it.

I know this is a bit confusing, the syntax for this will be improved in the next version of the library :)

[RichardCr]

Good to know thanks :D... ok now that I have all the correct error checking in place... it is the same deal. No new errors pop up, just the same two. compareObjectProps' can only compare objects [vuex-easy-firestore] Error! Something went wrong during the PATCH mutation: The document it's trying to patch does not exist. The document that permissions were removed from stays until a refresh when everything works fine again. So losing permissions to a document in a collection isn't triggering an error, or an update.

[louisameline]

The error handler will only tell you why the channel has been closed, and will not remove the local document for you, so you have to deal with it as you see fit (remove it, or make it read-only, etc.). Let us know if there is still an issue once you've taken the appropriate measures. Thank you!

[RichardCr]

Thanks for getting me this far. I'm trying to figure out where to deal with it which is tricky....

• The channel does not close and fails sliently so I am guessing none of the error handing there will be a place to handle this (I have also tried refreshed as in .then(({ refreshed, streaming }) => {...} but no errors there either.
• The permissions are removed using a cloud function, and are often removed by a different user so I don't have any event generated by the current user to hook into.
• The three hooks given by vuex-easy-firestore are for docs being added, modified or removed... None of those trigger unfortunately (which is maybe the issue? there is no hook like revoked for a document that has not been removed but is no longer accessible.. --> Feature Request :D
• vuex-easy-firestore gives the above errors and does not update the store so I can't put any logic in the store to handle it.

Are there any other places where I could detect this on the client ?

[louisameline]

I was mistaken, sorry. If the channel is on a document on which the user loses his read rights, the streaming promise will work as I said: you'll see the channel get closed. If it's on a collection however, if less documents become available because of removed rights, the channel should stay open and you should see them removed. You say it's not the case?

[RichardCr]

Thanks! You are correct, after more testing I have found I was incorrect. The removed hook does trigger... however only if I refresh the browser between the document being granted and then being revoked. The errors must have prevented the hook being fired. (The refresh prevents the errors being generated).. Sorry for the wild goose chase. I now have to figure out why revoking the document after a refresh would be different. The path is winding! Thanks so much for your help!

[RichardCr]

After going away and coming back and a lot more digging... things were not quite as they seemed! I fixed up the errors. They didn't change the behaviour unfortunately. So after a lot of console.log()'s later in vuex-easy-firestore/dist/index.esm.js here is where I am at. Any thoughts would be super helpful... On line 1458 var unsubscribe = dbRef.onSnapshot... I experience two different behaviours. When I remove the user's access to a document (using another user logged into an incog window).. depending on if there has been a refresh between the user being added or removed querySnapshot.metadata.fromCache is true or false. i.e. if I grant the user access then refresh the browser, then remove access querySnapshot.metadata.fromCache is false... without the refresh in-between it is true..

If it is false then the following giant switch goes from case 0 to case 1 where everything is nicely updated. If it is true the switch goes from case 0 to case 12 where nothing at all happens (as it assumes that the cache was updated by the user and thus in-sync with vuex ).

From here I am not too sure where to go. What has been updated in the cache isn't carried through to vuex... I don't really want to add any special casing to the switch as that thing is pretty complex! Any ideas? Thanks!!

[mesqueeb]

@RichardCr yeah, working with compiled files is always complex. In our source code we don't actually have those switches. It's much easier to work with and reason about the actual source code.

You can do this by locally downloading the source code for this library and then using a global npm library called yalc.

npm i -g yalc

then you cd into vuex-easy-firestore directory and you do

npm i
npm run rollup
yalc publish vuex-easy-firestore

then you cd into your working project's directory and you execute

yalc link vuex-easy-firestore

then any time you change something in vuex-easy-firestore source code, you just execute this again in that directory:

npm run rollup
yalc push

and your project will use your local version of vuex-easy-firestore while you are coding and debugging.

: )

As for your issue. This is a very specific case, probably hard to replicate. If you want to do a screen share session or a VSCode live session that's ok by me. We can try and find the source of the problem and how to best fix it.

Otherwise, I recommend using yalc to locally work with the source code of vuex-easy-firestore at your convenience. This will certainly be easier than working with the compiled dist/index.esm.js file!!

Good luck!!!

_{--
Vuex Easy Firestore was made with ♥ by Luca Ban.
If you use this library in your projects, you can support the maintenance of this library by a small contribution via Github 💜.
You can also reach out on twitter if you want a one-on-one coding review/lesson. 🦜}

[RichardCr]

Adding in this bit of code starting at line 1464 fixes my issue.

                            if (gotFirstLocalResponse) {
                                if (getters.collectionMode) {
                                    const docChanges = querySnapshot.docChanges();
                                    docChanges.forEach(function (change) {
                                        if (change.type === "removed"){
                                            var doc = getters.cleanUpRetrievedDoc(change.doc.data(), change.doc.id);
                                            dispatch('applyHooksAndUpdateState', {
                                                change: change.type,
                                                id: change.doc.id,
                                                doc: doc,
                                            });
                                        }
                                    });
                                }
                            }

I am not entirely sure what is happening, but my guess is that when permission to the document is revoked, firestore looks to the cached version, and at the same time realises that the document has been removed, then sends through the snapshot from the cache with change.type: 'removed'. A snapshot generated from cache that hasn't been generated by the user is an edgecase not taken into account on the opennDBChannel logic. Not sure why this behaviour doesn't happen if you refresh the page. I haven't managed to find much documentation on the intricacies of the firestore cache. If you are having trouble recreating / or understanding this edgecase to see if it warrants a bugfix I am happy to try and help.

[mesqueeb]

implemented! Thanks so much @RichardCr !!

[dsl101]

I think this may have had some unintended consequences... I've got a collection with over 100 user documents in it, and 2 modules using v-e-f.

The first module (fsUser) binds the currently logged-in user to the document at /users/{userId}. It can then test if the current user is an admin allowed to edit other user accounts. If so, it binds a second module (fsAllUsers) to the collection at /users.

But, what I'm seeing is that this second module only sees the current user document, not all 100+ users. And the reason is this change I think.

I added some logging to my local copy of the v-e-f package like this:

else if (gotFirstLocalResponse) {
    // it's not the first call and it's a change from cache
    // normally we only need to listen to the server changes, but there's an edge case here:
    // case: "a permission is removed server side, and instead of this being notified
    // by firestore from the _server side_, it only notifies this from the cache...
    if (getters.collectionMode) {
        docChanges = querySnapshot.docChanges();

console.log('docChanges:', docChanges)

        docChanges.forEach(function (change) {
            // only do stuff on "removed" !!
            if (change.type === 'removed') {
                var doc = getters.cleanUpRetrievedDoc(change.doc.data(), change.doc.id);
                dispatch('applyHooksAndUpdateState', {
                    change: change.type,
                    id: change.doc.id,
                    doc: doc,
                });
            }
        });
    }
}

and I can see in that console.log() line that there are 110 entries in docChanges. But then they are ignored, as they're all of type added...

I suspect this is because I already have the /users/{userId} document bound, and perhaps that is changing the behaviour of the firebase library? If I target a completely unrelated collection, I get all the docs as expected. But in this case, I do need to bind it twice in different modules (one as a doc, one as a collection).

If I comment out the line if (change.type === 'removed') { and its closing brace, I get all the docs in the collection.

However, I'm also missing updates on the collection module (e.g. changes in the bound document are synced with firestore, but then not replicated back to the bound collection), so perhaps this isn't the whole story. Should it be possible to have these 2 modules running happily side-by-side? If so, I'll try to make a minimal repro.

[mesqueeb]

@dsl101 I also have users and users/{userId} in one of my apps, and I noticed some oddness myself, but all stuff that's difficult to create automated tests for . : ( I noticed the problem about the missing updates, but I do get all docs loaded fine in the "users" collection.

This change however only does things on the "removed" hook though. So I'm confused why "added" is not working.