oortcloud / node-ddp-client

A callback style DDP (Meteor's Distributed Data Protocol) node client.
Other
263 stars 80 forks source link

Fix a connection bug, update README and example. #9

Closed AjayMT closed 11 years ago

AjayMT commented 11 years ago

I tried using the DDP client with the following code, and nothing happened.

var DDPClient = require('ddp');
var client = new DDPClient({ host: 'localhost', port:3000 });
client.connect(function () {
  console.log('connected');
});

I tried using client.on(), but it gave me an error saying that client has no attribute on. So I inspected the source code for a while, and then decided to try using client.socket.on(). The issue was that the server requires the client to specify the version of DDP that it is speaking. So now, the client specifies the version of DDP that it speaks (pre1 for now) and the versions of DDP it supports (['pre1'] for now), and establishes a connection.

tmeasday commented 11 years ago

Hi @AjayMT . Thanks for this.

I think there are two separate things here.

Firstly, is there an error in the documentation/examples? If so can you send a separate PR and we'll merge that ASAP.

Secondly, I don't think just sending pre1 is enough to ensure compatibility with the new DDP protocol. It might get past the handshake, but it'll break when you try to do any subscribing.

We plan to update to the new protocol (any word @sarfata ?), but haven't found the time yet. A PR doing this would be really appreciated!

emgee3 commented 11 years ago

@tmeasday, do you see different versions of DDP working in node-ddp-client moving forward? Would you like to the client supporting the latest version of DDP only, or auto-negotiating between all (or a subset) of different DDP versions?

tmeasday commented 11 years ago

Good question! I suppose negotiating would be ideal really. Personally, I only use it for atmosphere, and given there's only one atmosphere server, it's not a problem that I'll have.

But others will have different requirements no doubt.