dashersw / mogollar

A MongoDB UI built with Electron
MIT License
286 stars 29 forks source link

Connection only happens on the second click on "Connect" #12

Open dashersw opened 4 years ago

dashersw commented 4 years ago

A connection is established and the collections are fetched only on the second click to the Connect button. It should only take one click.

emredalkiran commented 4 years ago

The first connection to mongo daemon is immediately closed (without an error) right after a successful connection on the first click. The connection to mongo daemon does not close on the second click, thus collections can be retrieved successfully. I'm trying to narrow down the scope why the first connection gets closed.

emredalkiran commented 4 years ago

@dashersw When we first initiate the database connection, which is obviously from a renderer process, somehow this renderer process gets killed or view gets refreshed and this causes connection to mongo daemon to get closed (checked from mongod shell). At this stage, when I checked mongoose readyState, it still shows as if the connection is opened. Neither error, nor disconnect callbacks are fired. When the connect button is clicked for the second time, the connection remains open.

As an alternative, I've used inter process communication mechanism electron provides to initiate connection from main process. On button click for connection, I've sent a message to main process via ipcRenderer and received the message via ipcMain on background.js and I've seen that the connection persisted and did not get closed after first click.

I'm not sure if keeping database connection in main process and performing queries via main process causes blocking on UI. Even if keeping the connection in main process is reasonable, mongoose connection cannot be sent back to renderer process due to serialization problem. I've checked accessing store from main process in order to set state.connection but this is not possible by default mechanism of Vuex store, though some 3rd party npm packages provide (ex. https://github.com/vue-electron/vuex-electron) this flexibility.

I'm a bit stuck how to move forward from here on.

dashersw commented 4 years ago

Interesting! We needed to move this to the background process anyway, It shouldn't block the renderer process. We need a good way to manage this IPC events in a scalable manner, because event driven systems tend to get really messy over time. vuex-electron is a package that I use and like. Until we need IPC for other features, we can use vuex-electron and have the connection there and that would solve this issue.

emredalkiran commented 4 years ago

@dashersw I've tried assigning state.connection to the connection I create in main process with the help of vuex-electron package but serialization problem and circular structure problem prevented me from accomplishing that. This may be due to my lack of knowledge, I'm not sure.

The only possibility of making this work that I can think of is:

From your message above, I can understand that this is not how you are planning to move forward, at least until the point where IPC will be needed unavoidably.

dashersw commented 4 years ago

Oh sorry — so even though you don’t use anything on the renderer process, just because you set state.connection, even in the main process, it stops working?

Do we have to store the connection on the state? Can it be a free running variable? It looks like we won’t be able to avoid IPC at all? Worst case I’d love to have only one IPC call, instead of multiple, if possible.

emredalkiran commented 4 years ago

@dashersw I've solved the problem. For button element inside the form, no type attribute was set and it was defaulting to submit type, which as a result, was causing a page reload and tearing apart the first connection.

To clarify my previous attempt to solve this in another way; I was using ipc from renderer process to initiate database connection in the main process. After that, I needed to assign state.connection value to newly created connection that is only present in the main process. In order to do this, I've added to mutations and actions to the Vuex store. With the help of vuex-electron package, I tried to use store.commit and store.dispatch, but, these calls could not be executed due to serialization problem and circular reference problem (I was trying to pass mongoose.connection object). As I've checked several resources, in order to persist state of the store among processes, it needs to be updated via mutations, so I didn't try to modify state in the main process directly but via mutations and actions.

Imho, database connection should not be a free running variable. I was planning to keep it only in the main process and receive requests for database operations via ipc events, execute the query in the main process and update the state. So, connection was not going to be running freely. I honestly don't know if the connection should be stored in state. The main requirement here is to make connection(s) available to requests from different modules.

In any case, the bug was a very small issue but, I've traveled too far away to find it out.

dashersw commented 4 years ago

Sounds great. Can you open a PR with the fix? We can decide about the connection later.