-
Notifications
You must be signed in to change notification settings - Fork 8
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
Very nicely done #2
Comments
No I haven't tested with multiple browsers yet. My first suspicion would be that you are missing the clientIdentity (or that the server is doing something stupid with it). You can check the changes.table file saved on you disk to see if the changes have a 'source' property. Also check the meta.json file to see if it includes the client identities with their revisions. You would probably want to split up the sync/handler.js into two files one to react on client changes and one to send changes to the client when the server receives a change from another client. So basically you would use the Yes So I hope this helps you a little bit. PS: I'm not sure I the socket server example even supports multiple client connections at once. It seems to only have one connection object but I'm not familiar with websockets. |
@nponiros The changes.table
meta.json
Are you planning on testing multiple browsers anytime sooner. If so I'd rather leave the changes you suggest to you. |
Hmm |
FYI I'm just using two PC's but my little test app starts by deleting the Dexie database and then adds two documents. So I open the app in each PC/Browser and then refresh one, which deletes it's Dexie DB and then adds two doc's. Thanks for looking at this. |
@nevf So I did a quick test with 2 different browsers. Everything seems to be working fine. At least the server communication passed the data that I expected, the changes.table is correct and the nextClientID in the meta.json updates as expected. I had some issues with duplicated keys but that is probably an issue with my test data and/or my key generator on the client. After resetting everything (cleaning the server tables and IndexedDB in the browser) it worked without issues. |
@nponiros Thanks for trying, good to hear it works correctly. I'll continue working on my code and hopefully resolve my issue. |
Well I do need to write integration tests and more unit tests to make sure everything works correctly and reliably but it looks okay for now. But I can tell you that the socket example from Dexie is not well tested so there might be a bug or two there. If you don't manage to get it to work you can send me your code and I can have a look on Saturday. You can also create a PR if you want so that the sync-server has a sockets implementation. |
@nponiros When I use the WebSocketServer.js code it works correctly, but not when I use my code + your code. And I don't yet know where the issue lie. I've got limited time the next 2 days. Adding websockets support to sync-server would be great. I'll create a PR. I'm curious why you decided to use XHR and polling instead of sockets? |
Yes I used the logic from the socket server example but the function call order is differently and this might cause the issues you are seeing. I use polling because most of the time the server is offline and my app disconnects from the server so sockets would be useless. I use the sync-server locally on my laptop and not on a real server. |
@nponiros The I haven't made any further progress today, so if you can implement websockets in sync-server that would be great. I can send you my ws_server.js code, but you are probably better starting from scratch. |
@nevf just send me your code so I can get a feeling on how you are planing to use it before I implement something that might work completely different than what you expect/want. I can't give any guarantees on when I will be done with it but I will try to at least have a first version by Sunday. |
@nponiros Thanks for your help. I'm not in any great hurry, so please take your time. I've attached a zip file with my You will see various comments and TODO items. I think the main issue is the xhr code relies on each client requesting server changes, whereas the websocket implementation pushes changes out to all connected clients whenever they occur. I think I have that part working ok. However the websocket "subscribe" action isn't behaving as per the original A final comment. I feel that the data in Any questions please let me know. |
@nevf thanks for the code. I will have a look ASAP. Yes you are right that |
@nevf so I add a WebSocket server. It's not really tested yet but it might help you with your sync issues. I will try to test it later today. The implementation is in the socket_server branch. Edit: I made a small test using the socket sample I added to the repo. Things seem to work when I use 2 different browsers. I didn't test it with the socket sample from Dexie so it might not work with that yet. |
@nponiros Thanks for that. Your socket sample works fine on two PCs, however my sample app with Dexie.syncable still isn't. When I start the app on the second PC with an empty Dexie database, it isn't getting the existing changes from the server. New changes on either PC are synced correctly. Edit 1: I'll check my app code matches your sample, which may fix this. |
@nevf make sure that you get a client id before sending changes and subscribing. The socket protocol in the dexie samples assumes a synchronous client id generation (this is needed in case you are using the master branch. The socket server branch creates the id synchronously so it should work with the sample code). My samples get database changes but don't use syncable so I would say the server works. At least if the revision numbers are correct. If you want you can send me your client code and I will have a look or are you just using the socket sample from dexie without any other code? |
@nponiros |
@nponiros Ok from a quick test the client code
|
Are you using the master branch? In master this would not work. In the socket server branch it should work. For master you will have to wait for the response with type 'clientIdenty' before sending changes and subscribe. If you don't wait the server might not have enough time to associate the connection with the id. I will see if I find time tomorrow evening to test with the dexie sample. Yes my sample code does not use revisions because it doesn't save any data this shouldn't be an issue unless I have other bugs in thd codd. Btw the master branch does have some revision related fixes which might solve your issue. I will test with the dexie sample ASAP. |
@nponiros I am definitely using the socket_server branch. I didn't npm install it, but downloaded the zip file instead and then run it from node.js directly after installing all the node modules. Neither the I've attached a copy of my Thanks again for your efforts. |
@nevf so I found the mistake. I was using default params for
The above lines must be removed from the
This is due to the async generation of the PS: in the master branch the interaction with |
@nponiros Great and thanks. I'll get the updated master branch, make the change to |
@nponiros Ok looks great. The problem I was having where the 2nd PC wasn't getting changes on I assume you've seen the console error: Edit: This error only happens when a I delete the Dexie DB and therefore a new |
@nevf great to hear that.
The server needs some end to end tests. I'm not sure if the subscriptions should also be removed on error. My understanding is that error gets called if there was a transmition issue. I guess the client could try again although the dexie socket sample does stop syncing so removing the subscription might be the right thing to do. Do you have any insight on that? |
@nponiros You shouldn't do anything in |
@nevf okay, thanks for the input |
@nponiros I'm using sync-server with a change to use Websockets instead of http. This is based on the Dexie.js Websocket samples.
So far I've only had to replace your server.js with mine and much to my amazement it worked first time. You've done a great job of restructuring the Dexie sample code.
I am having a problem with specific changes not syncing when there are multiple clients and don't know yet whether this is a problem in my websocket server implementation or elsewhere. Have you tried sync-server with multiple clients (Browsers). ie. Add data in one and check it syncs correctly in the other one?
Also could you look at
sendAnyChanges()
in WebSocketSyncServer.js and let me know if yourgetServerChanges()
should match the behaviour ofsendAnyChanges()
. ref this comment:The XHR implementation does work a bit differently to the Websocket one which starts with a "subscribe" action to get any changes from the server that aren't in the client. This may be where I'm going wrong.
I'm calling
syncHandler()
when thesubscribe
message is received withsyncData
including thesyncedRevision
.The text was updated successfully, but these errors were encountered: