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

'database is destroyed' error while using 'withIdPrefix' #149

Closed
flootr opened this issue Apr 22, 2017 · 10 comments · Fixed by #153
Closed

'database is destroyed' error while using 'withIdPrefix' #149

flootr opened this issue Apr 22, 2017 · 10 comments · Fixed by #153

Comments

@flootr
Copy link
Contributor

flootr commented Apr 22, 2017

If I store a reference to a prefixed store I want to use throughout my application in its own file which I require from others, I get an database is destroyed-error after signing in or out.

I tried some things, but had no luck so far, for example using a new reference in every file with const prefixedStore = hoodie.store.withIdPrefix() get’s me a possible memory leak detected...-warning after using the application for a while.

Maybe I am misusing the method here, would appreciate some guidance then on how to improve :)

I created a test repo which should reflect both failure cases: flootr/hoodie-withidprefix-bug-test-case

@gr2m
Copy link
Member

gr2m commented Apr 23, 2017

Thanks @flootr! I didn’t yet figure out the problem but just wanted to let you know that I was able to reproduce it thanks to your repository, very much appreciated! This is on top of my Hoodie-things now, I’ll keep you posted. Mondays are usually rather busy, but hope to get to it on Tuesday or Wednesday latest

gr2m added a commit that referenced this issue Apr 26, 2017
gr2m added a commit that referenced this issue Apr 26, 2017
@gr2m
Copy link
Member

gr2m commented Apr 26, 2017

I created a test case for the bug you found:
#150

I’m most certain that the problem is this: .withIdPrefix() is a method from our PouchDB Plugin: https://github.com/hoodiehq/pouchdb-hoodie-api. It is bound to the current PouchDB instance. When we destroy the database of our PouchDB instance, all APIs returned by .withIdPrefix() are still bound to that instance. The logic to swap the old instance with a new one is inside the @hoodie/store-client module (this repository).

It will be tricky to fix, I have to give it some more thought

That’s how far I got so far

@gr2m
Copy link
Member

gr2m commented Jun 5, 2017

I tried to find a simple temporary workaround for this but it’s complicated ™️ becaues of who the implementation is split up between @hoodie/store-client and pouchdb-hoodie-api.

I’m tempted to deprecate pouchdb-hoodie-api altogether and move all the implementations of methods like .add, .update etc over to @hoodie/store-client. The only thing holding me back is that we use pouchdb-hoodie-api in @hoodie/store-server-api.

But we might simply add an option to @hoodie/store-client which would connect to a remote database directly and would not expose any of the sync APIs, as it’s something that we’ll need in the client API anyway.

So far my thoughts, sorry this takes so long. I hope it’s not blocking you

@flootr
Copy link
Contributor Author

flootr commented Jun 6, 2017

So far my thoughts, sorry this takes so long. I hope it’s not blocking you

Not at all 👍 I'm very grateful for what you're doing.

Right now it sounds like a bigger, more complicated change to me but if you think I can help at some point, ping me.

@gr2m
Copy link
Member

gr2m commented Jun 17, 2017

hey @flootr I prepared an issue with lots of context, would you like to give it a go yourself? hoodiehq/camp#121

@flootr
Copy link
Contributor Author

flootr commented Jun 18, 2017

@gr2m I saw you already claimed that issue which is really cool and I'm really looking forward to your solution 👍 🎉 😎

@gr2m
Copy link
Member

gr2m commented Jun 18, 2017

yeah sorry! I had a flight and that was perfect to make it a productive one. I’m ~50% through already

@gr2m
Copy link
Member

gr2m commented Jun 20, 2017

@flootr could you test it with this PR? #153

If you could review it, that’d be great, too :) Ask away all the questions you might have about it 👍

@flootr
Copy link
Contributor Author

flootr commented Jun 20, 2017

Yeah, of course 👍

@flootr
Copy link
Contributor Author

flootr commented Jun 20, 2017

@gr2m I wasn't able to reproduce the error by updating @hoodie/store-client in the bug-test-repo mentioned above 🎉

@gr2m gr2m closed this as completed in #153 Jun 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants