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

DDP Web Socket Reset Cycle in Meteor 0.8.1.1 #2125

Closed
vincilbishop opened this Issue May 7, 2014 · 5 comments

Comments

Projects
None yet
4 participants
@vincilbishop

vincilbishop commented May 7, 2014

DESCRIPTION

  • After upgrading to Meteor v0.8.1.1 iOS clients using the ObjectiveDDP library can no longer connect to the meteor server.
  • Downgrading the Meteor v0.8.0.1 allows those clients to again connect successfully.

Was there a change in the websocket implementation in Meteor v0.8.1.1?

A corresponding ObjectiveDDP issue has been created here: boundsj/ObjectiveDDP#74

PRE-REQUISITES

  1. XCode installed
  2. Cocoapods installed
  3. Meteorite installed

STEPS TO REPRODUCE

$ git clone https://github.com/premosystems/meteor-issue-2125
$ cd ./meteor-issue-2125/meteor-example-failing-0.8.1.1
$ mrt &
$ cd  ../ios-example
$ pod update
$ Open ios-example.xcworkspace

Run the iOS app in the simulator.

ACTUAL BEHAVIOR

meteor-example-failing-0.8.1.1

Observe that a "You are not connected" error is returned from the login call. The websocket then goes into a cycle of being ready and not ready.

2014-05-07 09:47:07.304 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 0
2014-05-07 09:47:07.306 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].connected: 0
2014-05-07 09:47:07.306 ios-example[95592:60b] [MYMeteorClient sharedClient].websocketReady: 0
2014-05-07 09:47:07.306 ios-example[95592:60b] [MYMeteorClient sharedClient].connected: 0
2014-05-07 09:47:07.307 ios-example[95592:60b] response: (null)
2014-05-07 09:47:07.307 ios-example[95592:60b] error: Error Domain=boundsj.objectiveddp.transport Code=0 "You are not connected" UserInfo=0x8f2b9a0 {NSLocalizedDescription=You are not connected}
2014-05-07 09:47:07.307 ios-example[95592:60b] websocketReady: 0
2014-05-07 09:47:11.335 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 1
2014-05-07 09:47:11.338 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 0
2014-05-07 09:47:11.339 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].connected: 0
2014-05-07 09:47:16.347 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 1
2014-05-07 09:47:16.349 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 0
2014-05-07 09:47:16.349 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].connected: 0
2014-05-07 09:47:21.358 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 1
2014-05-07 09:47:21.360 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 0
2014-05-07 09:47:21.360 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].connected: 0
2014-05-07 09:47:26.366 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 1
2014-05-07 09:47:26.368 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 0
2014-05-07 09:47:26.369 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].connected: 0
2014-05-07 09:47:31.375 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 1
2014-05-07 09:47:31.377 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 0
2014-05-07 09:47:31.377 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].connected: 0
2014-05-07 09:47:36.383 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 1
2014-05-07 09:47:36.386 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 0
2014-05-07 09:47:36.386 ios-example[95592:60b] RAC: [MYMeteorClient sharedClient].connected: 0

The websocketReady/NotReady cycle is caused by the SocketRocket delegate method below, called with a code of 1000 and a nil reason string. wasClean is true.

- (void)webSocket:(SRWebSocket *)webSocket didCloseWithCode:(NSInteger)code reason:(NSString *)reason wasClean:(BOOL)wasClean

ObjectiveDDP makes the websocket connection using the following command. Note the "pre1" protocol version. Is this the version that should be used with Meteor v0.8.1.1?

[self.ddp connectWithSession:nil version:@"pre1" support:nil];

EXPECTED BEHAVIOR

meteor-example-passing-0.8.0.1

To demonstrate expected behavior, execute the same steps to reproduce, except start meteorite from the meteor-example-passing-0.8.0.1 directory.

2014-05-07 09:44:51.491 ios-example[95415:60b] RAC: [MYMeteorClient sharedClient].websocketReady: 1
2014-05-07 09:44:51.492 ios-example[95415:60b] RAC: [MYMeteorClient sharedClient].connected: 1
2014-05-07 09:44:51.492 ios-example[95415:60b] [MYMeteorClient sharedClient].websocketReady: 1
2014-05-07 09:44:51.493 ios-example[95415:60b] [MYMeteorClient sharedClient].connected: 1
2014-05-07 09:44:51.497 ios-example[95415:60b] websocketReady: 1
2014-05-07 09:44:51.603 ios-example[95415:60b] response: {
    logon = success;
}
2014-05-07 09:44:51.603 ios-example[95415:60b] error: (null)
@vincilbishop

This comment has been minimized.

vincilbishop commented May 7, 2014

The ObjectiveDDP maintainer says that Meteor v0.8.1.1 requires a pre2 DDP protocol version instead of the hardcoded pre1. He says he is going to fix it right away.

@glasser

This comment has been minimized.

Member

glasser commented May 9, 2014

While on the one hand, the library maintainer should upgrade to use pre2, it's supposed to be backwards compatible with pre1.

It looks like the problem is support:nil. The support field should contain an array of all versions that the client supports in decreasing order of preference, ie, ["pre1"] or ["pre2", "pre1"]. Objective-DDP was essentially saying "while I want to be using pre1, I actually don't support any versions at all". The way the server implementation works, this happens to work if pre1 is the latest supported by the server, which is no longer true with 0.8.1.

So all of these things should happen:

  • Upgrade clients to pre2 :) That way they can do heartbeats (and randomSeed)!
  • ObjectiveDDP should send a support field.
  • The server should reject any request where the support field doesn't contain the version field; this would have caused the ObjectiveDDP library to not have this bug.

glasser added a commit that referenced this issue May 9, 2014

Stricter validation of connect messages
Previously, you could leave out the "support" field (or claim to not
support the version you were proposing) which would mean your connection
would succeed iff the proposed version is the server's favorite
version. This led to (eg) ObjectiveDDP accidentally writing a client
that stopped working when servers started preferring pre2 over pre1. By
making this a blatant error, DDP client libraries are more likely to
be written in a way that works with version negotiation.

Also, remove the delay in sending connect failure messages, which was
intended to avoid connect storms from clients that are by now 1.5 years
old.

Fixes #2125.
@jamesfebin

This comment has been minimized.

jamesfebin commented May 12, 2014

@glasser i am using it with pre2. It connects to server for 1 minute. Am able to retrive data. But then the next minute the connection gets disconnected. And this loop is happening over and over again.

@glasser

This comment has been minimized.

Member

glasser commented May 23, 2014

@jamesfebin This sounds like a different issue. I hope the good folks at https://github.com/boundsj/ObjectiveDDP can help you with it; if it turns out to be a Meteor issue, read https://github.com/meteor/meteor/wiki/Contributing-to-Meteor#reporting-a-bug-in-meteor and open a bug here.

@emgee3

This comment has been minimized.

Contributor

emgee3 commented May 24, 2014

@jamesfebin, just an idea: if you are proxying through nginx, I'm pretty sure it has a default websocket timeout of 60s.

nevyn added a commit to lookback/ObjectiveDDP that referenced this issue May 28, 2014

Send correct “support” array in connectWithSession
Sending nil is nonconformant and should result in a disconnection, because
the client is then saying “I don’t support any protocol versions at all”,
according to meteor/meteor#2125 .

Sending “support: [‘pre1’]” is a more correct fix than sending “version: pre2”,
since ObjectiveDDP doesn’t actually support pre2 yet. This is a more correct fix
to boundsj#74 .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment