From 42ff994a098d528966409fa3478ebbe0c4ac310f Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Mon, 24 Feb 2014 20:52:55 -0800 Subject: [PATCH 01/16] Avoid returning twice on error in `files.run` --- tools/files.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/files.js b/tools/files.js index 51ac720dd5..71dd92956e 100644 --- a/tools/files.js +++ b/tools/files.js @@ -478,9 +478,11 @@ files.run = function (command /*, arguments */) { var child_process = require("child_process"); child_process.execFile( command, args, {}, function (error, stdout, stderr) { - if (! (error === null || error.code === 0)) + if (! (error === null || error.code === 0)) { future.return(null); - future.return(stdout); + } else { + future.return(stdout); + } }); return future.wait(); }; From 729f4123f1b8967302c6aef824593fcfa78ab71a Mon Sep 17 00:00:00 2001 From: David Glasser Date: Wed, 26 Feb 2014 13:34:24 -0800 Subject: [PATCH 02/16] Only set oplog-reply flag on the oplog collection Otherwise can crash mongod! Wed Feb 26 11:09:32.829 [conn499] logs.tmp-test-logs-10833_meteor_com Assertion failure str::startsWith(ns, "local.oplog.") src/mongo/db/repl_block.cpp 261 0x10b9245ab 0x10b8fb64c 0x10b816015 0x10b5b2d06 0x10b714ce6 0x10b6bf354 0x10b6c398b 0x10b4dd9e2 0x10b917699 0x10b957055 0x7fff948637a2 0x7fff948501e1 I20140226-11:09:32.831(-8) (satellite.js:296) process 85850 for job tmp-test-logs-10833.meteor.com null exited with 1 0 mongod 0x000000010b9245ab _ZN5mongo15printStackTraceERSo + 43 1 mongod 0x000000010b8fb64c _ZN5mongo12verifyFailedEPKcS1_j + 284 2 mongod 0x000000010b816015 _ZN5mongo19updateSlaveLocationERNS_5CurOpEPKcNS_6OpTimeE + 2405 3 mongod 0x000000010b5b2d06 _ZN5mongo12ClientCursor19updateSlaveLocationERNS_5CurOpE + 52 4 mongod 0x000000010b714ce6 _ZN5mongo14processGetMoreEPKcixRNS_5CurOpEiRbPb + 534 5 mongod 0x000000010b6bf354 _ZN5mongo15receivedGetMoreERNS_10DbResponseERNS_7MessageERNS_5CurOpE + 1492 6 mongod 0x000000010b6c398b _ZN5mongo16assembleResponseERNS_7MessageERNS_10DbResponseERKNS_11HostAndPortE + 4939 7 mongod 0x000000010b4dd9e2 _ZN5mongo16MyMessageHandler7processERNS_7MessageEPNS_21AbstractMessagingPortEPNS_9LastErrorE + 198 8 mongod 0x000000010b917699 _ZN5mongo17PortMessageServer17handleIncomingMsgEPv + 1657 9 mongod 0x000000010b957055 thread_proxy + 229 10 libsystem_c.dylib 0x00007fff948637a2 _pthread_start + 327 11 libsystem_c.dylib 0x00007fff948501e1 thread_start + 13 --- packages/mongo-livedata/mongo_driver.js | 12 ++++++++---- packages/mongo-livedata/oplog_tailing.js | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/mongo-livedata/mongo_driver.js b/packages/mongo-livedata/mongo_driver.js index b3469a09be..caf1c927fb 100644 --- a/packages/mongo-livedata/mongo_driver.js +++ b/packages/mongo-livedata/mongo_driver.js @@ -754,11 +754,15 @@ MongoConnection.prototype._createSynchronousCursor = function( // ... and to keep querying the server indefinitely rather than just 5 times // if there's no more data. mongoOptions.numberOfRetries = -1; - // And if this cursor specifies a 'ts', then set the undocumented oplog - // replay flag, which does a special scan to find the first document - // (instead of creating an index on ts). - if (cursorDescription.selector.ts) + // And if this is on the oplog collection and the cursor specifies a 'ts', + // then set the undocumented oplog replay flag, which does a special scan to + // find the first document (instead of creating an index on ts). This is a + // very hard-coded Mongo flag which only works on the oplog collection and + // only works with the ts field. + if (cursorDescription.collectionName === OPLOG_COLLECTION && + cursorDescription.selector.ts) { mongoOptions.oplogReplay = true; + } } var dbCursor = collection.find( diff --git a/packages/mongo-livedata/oplog_tailing.js b/packages/mongo-livedata/oplog_tailing.js index b8c044ce3e..b203bfa412 100644 --- a/packages/mongo-livedata/oplog_tailing.js +++ b/packages/mongo-livedata/oplog_tailing.js @@ -1,6 +1,6 @@ var Future = Npm.require('fibers/future'); -var OPLOG_COLLECTION = 'oplog.rs'; +OPLOG_COLLECTION = 'oplog.rs'; var REPLSET_COLLECTION = 'system.replset'; // Like Perl's quotemeta: quotes all regexp metacharacters. See From 0eccd1f2f49f674ba6e9f31a378fa17bc205d3df Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 26 Feb 2014 13:51:38 -0800 Subject: [PATCH 03/16] Add History entries for 0.7.1.2 bug fixes --- History.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/History.md b/History.md index c1e0c626b7..699137d47c 100644 --- a/History.md +++ b/History.md @@ -1,6 +1,16 @@ ## v.NEXT -## v0.7.1 + +## v0.7.1.2 + +* Fix bug in tool error handling that caused `meteor` to crash on Mac + OSX when no computer name is set. + +* Work around a bug that caused MongoDB to fail an assertion when using + tailable cursors on non-oplog collections. + + +## v0.7.1.1 * Integrate with Meteor developer accounts, a new way of managing your meteor.com deployed sites. When you use `meteor deploy`, you will be From 3caa3b9e9c4cb1741034e07e4d50f300c4dbefaf Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 26 Feb 2014 13:51:58 -0800 Subject: [PATCH 04/16] Update banner and notices for 0.7.1.2 --- scripts/admin/banner.txt | 7 ++----- scripts/admin/notices.json | 3 +++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/admin/banner.txt b/scripts/admin/banner.txt index a2c290ebd1..e672e9ae21 100644 --- a/scripts/admin/banner.txt +++ b/scripts/admin/banner.txt @@ -1,7 +1,4 @@ -=> Meteor 0.7.1: Extend oplog tailing driver to support most common - MongoDB queries. Introduce Meteor developer accounts, a new way of - managing your meteor.com deployed sites. When you use `meteor - deploy`, you will be prompted to create a developer account. +=> Meteor 0.7.1.2: Fix crash on OSX machines with no hostname set. This release is being downloaded in the background. Update your - project to Meteor 0.7.1 by running 'meteor update'. + project to Meteor 0.7.1.2 by running 'meteor update'. diff --git a/scripts/admin/notices.json b/scripts/admin/notices.json index a0c2026c00..9c26c504a6 100644 --- a/scripts/admin/notices.json +++ b/scripts/admin/notices.json @@ -85,6 +85,9 @@ "http://jquery.com/upgrade-guide/1.9/"] } }, + { + "release": "0.7.1.2" + }, { "release": "NEXT" } From b0d86e535c394d1725f7b9ba6692e5587375325f Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 26 Feb 2014 17:48:17 -0800 Subject: [PATCH 05/16] Update docs and examples --- docs/.meteor/release | 2 +- docs/lib/release-override.js | 2 +- examples/leaderboard/.meteor/release | 2 +- examples/parties/.meteor/release | 2 +- examples/todos/.meteor/release | 2 +- examples/wordplay/.meteor/release | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/.meteor/release b/docs/.meteor/release index da0ead2e23..5a848a1d77 100644 --- a/docs/.meteor/release +++ b/docs/.meteor/release @@ -1 +1 @@ -0.7.1-rc3 +0.7.1.2 diff --git a/docs/lib/release-override.js b/docs/lib/release-override.js index 2ea0eedc8c..523bf17b11 100644 --- a/docs/lib/release-override.js +++ b/docs/lib/release-override.js @@ -1,5 +1,5 @@ // While galaxy apps are on their own special meteor releases, override // Meteor.release here. if (Meteor.isClient) { - Meteor.release = Meteor.release ? "0.7.1" : undefined; + Meteor.release = Meteor.release ? "0.7.1.2" : undefined; } diff --git a/examples/leaderboard/.meteor/release b/examples/leaderboard/.meteor/release index da0ead2e23..5a848a1d77 100644 --- a/examples/leaderboard/.meteor/release +++ b/examples/leaderboard/.meteor/release @@ -1 +1 @@ -0.7.1-rc3 +0.7.1.2 diff --git a/examples/parties/.meteor/release b/examples/parties/.meteor/release index da0ead2e23..5a848a1d77 100644 --- a/examples/parties/.meteor/release +++ b/examples/parties/.meteor/release @@ -1 +1 @@ -0.7.1-rc3 +0.7.1.2 diff --git a/examples/todos/.meteor/release b/examples/todos/.meteor/release index da0ead2e23..5a848a1d77 100644 --- a/examples/todos/.meteor/release +++ b/examples/todos/.meteor/release @@ -1 +1 @@ -0.7.1-rc3 +0.7.1.2 diff --git a/examples/wordplay/.meteor/release b/examples/wordplay/.meteor/release index da0ead2e23..5a848a1d77 100644 --- a/examples/wordplay/.meteor/release +++ b/examples/wordplay/.meteor/release @@ -1 +1 @@ -0.7.1-rc3 +0.7.1.2 From 825082b3a81051f4c6d45fd698c6fbd4d8711da8 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 26 Feb 2014 23:49:18 -0800 Subject: [PATCH 06/16] Add missing semicolon in deploy-galaxy --- tools/deploy-galaxy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/deploy-galaxy.js b/tools/deploy-galaxy.js index 6095545a7b..c0aa0fc808 100644 --- a/tools/deploy-galaxy.js +++ b/tools/deploy-galaxy.js @@ -78,7 +78,7 @@ var ServiceConnection = function (galaxy, service) { // from the hostname of endpointUrl, and run the login command for // that galaxy. if (! authToken) - throw new Error("not logged in to galaxy?") + throw new Error("not logged in to galaxy?"); self.connection = Package.livedata.DDP.connect(endpointUrl, { headers: { From 35c1a5fc457c6f2dd747875498420c959b719d0d Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Wed, 26 Feb 2014 23:57:18 -0800 Subject: [PATCH 07/16] Replace `subscribeAndWait` with `subscribe` inside `onReconnect` --- tools/deploy-galaxy.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tools/deploy-galaxy.js b/tools/deploy-galaxy.js index c0aa0fc808..945eef38ef 100644 --- a/tools/deploy-galaxy.js +++ b/tools/deploy-galaxy.js @@ -462,9 +462,17 @@ exports.logs = function (options) { var opts = { streaming: options.streaming }; if (lastLogId) opts.resumeAfterId = lastLogId; + // Don't use subscribeAndWait here; it'll deadlock. We can't + // process the sub messages until `onReconnect` returns, and + // `onReconnect` won't return unless the sub messages have been + // processed. There's no reason we need to wait for the sub to be + // ready here anyway. // XXX correctly handle errors on resubscribe - logsSubscription = logReader.subscribeAndWait("logsForApp", - options.app, opts); + logsSubscription = logReader.connection.subscribe( + "logsForApp", + options.app, + opts + ); }; try { From 9d5782b9a064255a32c5b91e5193813dc8a73c59 Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Thu, 27 Feb 2014 07:56:19 -0800 Subject: [PATCH 08/16] Return the sub from `ServiceConnection.subscribeAndWait` --- tools/deploy-galaxy.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/deploy-galaxy.js b/tools/deploy-galaxy.js index 945eef38ef..d79ef34a31 100644 --- a/tools/deploy-galaxy.js +++ b/tools/deploy-galaxy.js @@ -151,8 +151,9 @@ _.extend(ServiceConnection.prototype, { } }); - self.connection.subscribe.apply(self.connection, args); - return fut.wait(); + var sub = self.connection.subscribe.apply(self.connection, args); + fut.wait(); + return sub; }, close: function () { From bc4524b54410379be0455c19d0bd1668a3cb3c3b Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Thu, 27 Feb 2014 07:56:36 -0800 Subject: [PATCH 09/16] Set up onReconnect after initial sub on the connection to log-reader. If we set it up before `subscribeAndWait` returns, then we'll end up with two subscriptions; we don't have the log-reader sub yet, so we can't stop it when `onReconnect` runs the first time, so we end up with a redundant subscription. This means that if a real reconnect happens later, we'll stop the sub that we set up inside `onReconnect`, but not the initial sub, so we've leaked a sub and end up with duplicate messages after reconnect. --- tools/deploy-galaxy.js | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/tools/deploy-galaxy.js b/tools/deploy-galaxy.js index d79ef34a31..d879bb6871 100644 --- a/tools/deploy-galaxy.js +++ b/tools/deploy-galaxy.js @@ -457,7 +457,21 @@ exports.logs = function (options) { throw new Error("Can't listen to messages on the logs collection"); var logsSubscription = null; - // In case of reconnect recover the state so user sees only new logs + try { + logsSubscription = + logReader.subscribeAndWait("logsForApp", options.app, + { streaming: options.streaming }); + } catch (e) { + return handleError(e, galaxy, { + "no-such-app": "No such app: " + options.app + }); + } + + // In case of reconnect recover the state so user sees only new logs. + // Only set up the onReconnect handler after the subscribe and wait + // has returned; if we set it up before, then we'll end up with two + // subscriptions, because the onReconnect handler will run for the + // first time before the subscribeAndWait returns. logReader.connection.onReconnect = function () { logsSubscription && logsSubscription.stop(); var opts = { streaming: options.streaming }; @@ -476,16 +490,6 @@ exports.logs = function (options) { ); }; - try { - logsSubscription = - logReader.subscribeAndWait("logsForApp", options.app, - { streaming: options.streaming }); - } catch (e) { - return handleError(e, galaxy, { - "no-such-app": "No such app: " + options.app - }); - } - return options.streaming ? null : 0; } finally { // If not streaming, close the connection to log-reader so that From 9c1dc8782cbd2c535261216073f0bf9a178dd42f Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Thu, 27 Feb 2014 08:09:31 -0800 Subject: [PATCH 10/16] Clean up ServiceConnection timer when we receive a result. Previously, we could make a connection, do some method calls, and then 10 seconds later the connection happens to be dropped and the connection timer fires, which not only throws an unexpected error into the future, but also resolves the future twice. I think ServiceConnection is just supposed to time out if you don't hear anything from the server within 10 seconds, so it now no longer times out if you hear things from the server but then happen to be not connected when 10 seconds has elapsed. --- tools/deploy-galaxy.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tools/deploy-galaxy.js b/tools/deploy-galaxy.js index d879bb6871..53319a1379 100644 --- a/tools/deploy-galaxy.js +++ b/tools/deploy-galaxy.js @@ -117,10 +117,12 @@ _.extend(ServiceConnection.prototype, { var args = _.toArray(arguments); var name = args.shift(); self.connection.apply(name, args, function (err, result) { - if (err) + if (err) { fut['throw'](err); - else + } else { + self._cleanUpTimer(); fut['return'](result); + } }); return fut.wait(); @@ -141,6 +143,7 @@ _.extend(ServiceConnection.prototype, { args.push({ onReady: function () { ready = true; + self._cleanUpTimer(); fut['return'](); }, onError: function (e) { @@ -156,6 +159,13 @@ _.extend(ServiceConnection.prototype, { return sub; }, + _cleanUpTimer: function () { + var self = this; + var Package = getPackage(); + Package.meteor.Meteor.clearTimeout(self.connectionTimer); + self.connectionTimer = null; + }, + close: function () { var self = this; if (self.connection) { @@ -164,9 +174,7 @@ _.extend(ServiceConnection.prototype, { } if (self.connectionTimer) { // Clean up the timer so that Node can exit cleanly - var Package = getPackage(); - Package.meteor.Meteor.clearTimeout(self.connectionTimer); - self.connectionTimer = null; + self._cleanUpTimer(); } } }); From d049bf75063e952e02b7bd719f575072147071e8 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 24 Feb 2014 20:22:49 -0800 Subject: [PATCH 11/16] Use faye-websocket for server-to-server DDP This matches what the SockJS server uses; now we only need to understand and fix bugs in the implementation of one websocket npm module. Some notes: - I actually trust that it's possible to close a connection before it successfully connects, which allows me to simplify the code a lot (since there shouldn't be multiple connections active per ClientStream). I put in some assertions to make sure this is the case, though. (Note that this module also has a simpler model, where there's a single object representing the client connection, not a "client" object that spawns "connections".) - We now print connect errors as well as post-connect errors. (This required adding a flag to keep tests quiet since it makes an expected-to-fail-to-connect connection.) We need a better approach to stream error handling, though. - We used to have a test to make sure that a certain not-user-visible callback is called within a Fiber; structuring the code such that this test is still possible would lead to the code being less consistent and harder to read, so I dropped the test. - Fix a few bugs where we weren't using Meteor.setTimeout. --- History.md | 4 + .../livedata/.npm/package/npm-shrinkwrap.json | 9 +- packages/livedata/livedata_connection.js | 5 +- packages/livedata/livedata_server.js | 2 +- packages/livedata/livedata_tests.js | 3 +- packages/livedata/package.js | 6 +- packages/livedata/stream_client_nodejs.js | 194 ++++++++---------- packages/livedata/stream_client_tests.js | 37 ++-- packages/retry/retry.js | 2 +- 9 files changed, 127 insertions(+), 135 deletions(-) diff --git a/History.md b/History.md index 513267f88c..15d33efebc 100644 --- a/History.md +++ b/History.md @@ -1,5 +1,9 @@ ## v.NEXT +* Use "faye-websocket" (0.7.2) npm module instead of "websocket" (1.0.8) for + server-to-server DDP. + + ## v0.7.1.2 * Fix bug in tool error handling that caused `meteor` to crash on Mac diff --git a/packages/livedata/.npm/package/npm-shrinkwrap.json b/packages/livedata/.npm/package/npm-shrinkwrap.json index f48e03a46b..92d8fd541c 100644 --- a/packages/livedata/.npm/package/npm-shrinkwrap.json +++ b/packages/livedata/.npm/package/npm-shrinkwrap.json @@ -16,8 +16,13 @@ } } }, - "websocket": { - "version": "1.0.8" + "faye-websocket": { + "version": "0.7.2", + "dependencies": { + "websocket-driver": { + "version": "0.3.2" + } + } } } } diff --git a/packages/livedata/livedata_connection.js b/packages/livedata/livedata_connection.js index 30ff1ef377..be5b79e15c 100644 --- a/packages/livedata/livedata_connection.js +++ b/packages/livedata/livedata_connection.js @@ -49,7 +49,10 @@ var Connection = function (url, options) { self._stream = new LivedataTest.ClientStream(url, { retry: options.retry, headers: options.headers, - _sockjsOptions: options._sockjsOptions + _sockjsOptions: options._sockjsOptions, + // To keep some tests quiet (because we don't have a real API for handling + // client-stream-level errors). + _dontPrintErrors: options._dontPrintErrors }); } diff --git a/packages/livedata/livedata_server.js b/packages/livedata/livedata_server.js index 95ddab9e6f..d579180911 100644 --- a/packages/livedata/livedata_server.js +++ b/packages/livedata/livedata_server.js @@ -1160,7 +1160,7 @@ _.extend(Server.prototype, { // drop all future data coming over this connection on the // floor. We don't want to confuse things. socket.removeAllListeners('data'); - setTimeout(function () { + Meteor.setTimeout(function () { socket.send(stringifyDDP({msg: 'failed', version: version})); socket.close(); }, timeout); diff --git a/packages/livedata/livedata_tests.js b/packages/livedata/livedata_tests.js index 65736d76f8..07bd42e779 100644 --- a/packages/livedata/livedata_tests.js +++ b/packages/livedata/livedata_tests.js @@ -683,9 +683,10 @@ if (Meteor.isServer) { testAsyncMulti("livedata - connect fails to unknown place", [ function (test, expect) { var self = this; - self.conn = DDP.connect("example.com"); + self.conn = DDP.connect("example.com", {_dontPrintErrors: true}); Meteor.setTimeout(expect(function () { test.isFalse(self.conn.status().connected, "Not connected"); + self.conn.close(); }), 500); } ]); diff --git a/packages/livedata/package.js b/packages/livedata/package.js index d2b407ae13..0e585b8467 100644 --- a/packages/livedata/package.js +++ b/packages/livedata/package.js @@ -3,7 +3,11 @@ Package.describe({ internal: true }); -Npm.depends({sockjs: "0.3.8", websocket: "1.0.8"}); +// We use 'faye-websocket' for connections in server-to-server DDP, mostly +// because it's the same library used as a server in sockjs, and it's easiest to +// deal with a single websocket implementation. (Plus, its maintainer is easy +// to work with on pull requests.) +Npm.depends({sockjs: "0.3.8", "faye-websocket": "0.7.2"}); Package.on_use(function (api) { api.use(['check', 'random', 'ejson', 'json', 'underscore', 'deps', diff --git a/packages/livedata/stream_client_nodejs.js b/packages/livedata/stream_client_nodejs.js index 708189deea..a3bc996d27 100644 --- a/packages/livedata/stream_client_nodejs.js +++ b/packages/livedata/stream_client_nodejs.js @@ -11,43 +11,16 @@ // ping frames or with DDP-level messages.) LivedataTest.ClientStream = function (endpoint, options) { var self = this; + options = options || {}; + self.options = _.extend({ retry: true }, options); - // WebSocket-Node https://github.com/Worlize/WebSocket-Node - // Chosen because it can run without native components. It has a - // somewhat idiosyncratic API. We may want to use 'ws' instead in the - // future. - // - // Since server-to-server DDP is still an experimental feature, we only - // require the module if we actually create a server-to-server - // connection. This is a minor efficiency improvement, but moreover: while - // 'websocket' doesn't require native components, it tries to use some - // optional native components and prints a warning if it can't load - // them. Since native components in packages don't work when transferred to - // other architectures yet, this means that require('websocket') prints a - // spammy log message when deployed to another architecture. Delaying the - // require means you only get the log message if you're actually using the - // feature. - self.client = new (Npm.require('websocket').client)(); + self.client = null; // created in _launchConnection self.endpoint = endpoint; - self.currentConnection = null; - options = options || {}; - self.headers = options.headers || {}; - - self.client.on('connect', Meteor.bindEnvironment( - function (connection) { - return self._onConnect(connection); - }, - "stream connect callback" - )); - - self.client.on('connectFailed', function (error) { - // XXX: Make this do something better than make the tests hang if it does not work. - return self._lostConnection(); - }); + self.headers = self.options.headers || {}; self._initCommon(); @@ -63,7 +36,7 @@ _.extend(LivedataTest.ClientStream.prototype, { send: function (data) { var self = this; if (self.currentStatus.connected) { - self.currentConnection.send(data); + self.client.send(data); } }, @@ -73,80 +46,37 @@ _.extend(LivedataTest.ClientStream.prototype, { self.endpoint = url; }, - _onConnect: function (connection) { + _onConnect: function (client) { var self = this; + if (client !== self.client) { + // This connection is not from the last call to _launchConnection. + // But _launchConnection calls _cleanup which closes previous connections. + // It's our belief that this stifles future 'open' events, but maybe + // we are wrong? + throw new Error("Got open from inactive client"); + } + if (self._forcedToDisconnect) { // We were asked to disconnect between trying to open the connection and // actually opening it. Let's just pretend this never happened. - connection.close(); + self.client.close(); + self.client = null; return; } if (self.currentStatus.connected) { - // We already have a connection. It must have been the case that - // we started two parallel connection attempts (because we - // wanted to 'reconnect now' on a hanging connection and we had - // no way to cancel the connection attempt.) Just ignore/close - // the latecomer. - connection.close(); - return; + // We already have a connection. It must have been the case that we + // started two parallel connection attempts (because we wanted to + // 'reconnect now' on a hanging connection and we had no way to cancel the + // connection attempt.) But this shouldn't happen (similarly to the client + // !== self.client check above). + throw new Error("Two parallel connections?"); } - if (self.connectionTimer) { - clearTimeout(self.connectionTimer); - self.connectionTimer = null; - } - - var onError = Meteor.bindEnvironment( - function (_this, error) { - if (self.currentConnection !== _this) - return; - - Meteor._debug("stream error", error.toString(), - (new Date()).toDateString()); - self._lostConnection(); - }, - "stream error callback" - ); - - connection.on('error', function (error) { - // We have to pass in `this` explicitly because bindEnvironment - // doesn't propagate it for us. - onError(this, error); - }); - - var onClose = Meteor.bindEnvironment( - function (_this) { - if (self.options._testOnClose) - self.options._testOnClose(); - - if (self.currentConnection !== _this) - return; - - self._lostConnection(); - }, - "stream close callback" - ); - - connection.on('close', function () { - // We have to pass in `this` explicitly because bindEnvironment - // doesn't propagate it for us. - onClose(this); - }); - - connection.on('message', function (message) { - if (self.currentConnection !== this) - return; // old connection still emitting messages - - if (message.type === "utf8") // ignore binary frames - _.each(self.eventCallbacks.message, function (callback) { - callback(message.utf8Data); - }); - }); + self._clearConnectionTimer(); // update status - self.currentConnection = connection; self.currentStatus.status = "connected"; self.currentStatus.connected = true; self.currentStatus.retryCount = 0; @@ -161,10 +91,10 @@ _.extend(LivedataTest.ClientStream.prototype, { var self = this; self._clearConnectionTimer(); - if (self.currentConnection) { - var conn = self.currentConnection; - self.currentConnection = null; - conn.close(); + if (self.client) { + var client = self.client; + self.client = null; + client.close(); } }, @@ -181,25 +111,63 @@ _.extend(LivedataTest.ClientStream.prototype, { var self = this; self._cleanup(); // cleanup the old socket, if there was one. - // launch a connect attempt. we have no way to track it. we either - // get an _onConnect event, or we don't. + // Since server-to-server DDP is still an experimental feature, we only + // require the module if we actually create a server-to-server + // connection. + var FayeWebSocket = Npm.require('faye-websocket'); - // XXX: set up a timeout on this. + // We would like to specify 'ddp' as the subprotocol here. The npm module we + // used to use as a client would fail the handshake if we ask for a + // subprotocol and the server doesn't send one back (and sockjs doesn't). + // Faye doesn't have that behavior; it's unclear from reading RFC 6455 if + // Faye is erroneous or not. So for now, we don't specify protocols. + var client = self.client = new FayeWebSocket.Client( + toWebsocketUrl(self.endpoint), + [/*no subprotocols*/], + {headers: self.headers} + ); - // we would like to specify 'ddp' as the protocol here, but - // unfortunately WebSocket-Node fails the handshake if we ask for - // a protocol and the server doesn't send one back (and sockjs - // doesn't). also, related: I guess we have to accept that - // 'stream' is ddp-specific - self.client.connect(toWebsocketUrl(self.endpoint), - undefined, // protocols - undefined, // origin - self.headers); - - if (self.connectionTimer) - clearTimeout(self.connectionTimer); - self.connectionTimer = setTimeout( + self._clearConnectionTimer(); + self.connectionTimer = Meteor.setTimeout( _.bind(self._lostConnection, self), self.CONNECT_TIMEOUT); + + self.client.on('open', Meteor.bindEnvironment(function () { + return self._onConnect(client); + }, "stream connect callback")); + + var clientOnIfCurrent = function (event, description, f) { + self.client.on(event, Meteor.bindEnvironment(function () { + // Ignore events from any connection we've already cleaned up. + if (client !== self.client) + return; + f.apply(this, arguments); + }, description)); + }; + + clientOnIfCurrent('error', 'stream error callback', function (error) { + if (!self.options._dontPrintErrors) + Meteor._debug("stream error", error.message); + + // XXX: Make this do something better than make the tests hang if it does + // not work. + self._lostConnection(); + }); + + + clientOnIfCurrent('close', 'stream close callback', function () { + self._lostConnection(); + }); + + + clientOnIfCurrent('message', 'stream message callback', function (message) { + // Ignore binary frames, where message.data is a Buffer + if (typeof message.data !== "string") + return; + + _.each(self.eventCallbacks.message, function (callback) { + callback(message.data); + }); + }); } }); diff --git a/packages/livedata/stream_client_tests.js b/packages/livedata/stream_client_tests.js index dbb675852d..d9402a2157 100644 --- a/packages/livedata/stream_client_tests.js +++ b/packages/livedata/stream_client_tests.js @@ -1,17 +1,24 @@ var Fiber = Npm.require('fibers'); -Tinytest.addAsync("stream client - callbacks run in a fiber", function (test, onComplete) { - stream = new LivedataTest.ClientStream( - Meteor.absoluteUrl(), - { - _testOnClose: function () { - test.isTrue(Fiber.current); - onComplete(); - } - } - ); - stream.on('reset', function () { - test.isTrue(Fiber.current); - stream.disconnect(); - }); -}); +testAsyncMulti("stream client - callbacks run in a fiber", [ + function (test, expect) { + var stream = new LivedataTest.ClientStream(Meteor.absoluteUrl()); + + var messageFired = false; + var resetFired = false; + + stream.on('message', expect(function () { + test.isTrue(Fiber.current); + if (resetFired) + stream.disconnect(); + messageFired = true; + })); + + stream.on('reset', expect(function () { + test.isTrue(Fiber.current); + if (messageFired) + stream.disconnect(); + resetFired = true; + })); + } +]); diff --git a/packages/retry/retry.js b/packages/retry/retry.js index a4407b5bdb..4a377bca4f 100644 --- a/packages/retry/retry.js +++ b/packages/retry/retry.js @@ -57,7 +57,7 @@ _.extend(Retry.prototype, { var timeout = self._timeout(count); if (self.retryTimer) clearTimeout(self.retryTimer); - self.retryTimer = setTimeout(fn, timeout); + self.retryTimer = Meteor.setTimeout(fn, timeout); return timeout; } From dcf806f852dcc6e9a6041b1d9a8f028abbf7e581 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Fri, 28 Feb 2014 11:52:44 -0800 Subject: [PATCH 12/16] typo in history --- History.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/History.md b/History.md index 15d33efebc..f96b92f149 100644 --- a/History.md +++ b/History.md @@ -42,7 +42,7 @@ * Add and improve support for minimongo operators. - Support `$comment`. - Support `obj` name in `$where`. - - `$regexp` matches actual regexps properly. + - `$regex` matches actual regexps properly. - Improve support for `$nin`, `$ne`, `$not`. - Support using `{ $in: [/foo/, /bar/] }`. #1707 - Support `{$exists: false}`. From 45db64d4bf8f994fa53f6d50a4f1f480c8eeaf93 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Fri, 28 Feb 2014 11:58:29 -0800 Subject: [PATCH 13/16] Support {a: {$regex: '', $options: 'i'}} Fixes #1874. --- packages/minimongo/minimongo_tests.js | 3 +++ packages/minimongo/selector.js | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index ef2fed60b1..dd8dc6de94 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -678,6 +678,9 @@ Tinytest.add("minimongo - selector_compiler", function (test) { nomatch({a: {$regex: 'a'}}, {a: 'cut'}); nomatch({a: {$regex: 'a'}}, {a: 'CAT'}); match({a: {$regex: 'a', $options: 'i'}}, {a: 'CAT'}); + match({a: {$regex: '', $options: 'i'}}, {a: 'foo'}); + nomatch({a: {$regex: '', $options: 'i'}}, {}); + nomatch({a: {$regex: '', $options: 'i'}}, {a: 5}); nomatch({a: /undefined/}, {}); nomatch({a: {$regex: 'undefined'}}, {}); nomatch({a: /xxx/}, {}); diff --git a/packages/minimongo/selector.js b/packages/minimongo/selector.js index 87abd1ca27..3b68a320f9 100644 --- a/packages/minimongo/selector.js +++ b/packages/minimongo/selector.js @@ -384,7 +384,7 @@ var VALUE_OPERATORS = { }, // $options just provides options for $regex; its logic is inside $regex $options: function (operand, valueSelector) { - if (!valueSelector.$regex) + if (!_.has(valueSelector, '$regex')) throw Error("$options needs a $regex"); return everythingMatcher; }, From 3ebc9aba60fea68a095aec6b4f2adb51ce46e6a4 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Fri, 28 Feb 2014 11:59:03 -0800 Subject: [PATCH 14/16] arguably support empty regexs in file watcher --- tools/watch.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/watch.js b/tools/watch.js index f71cb7805c..a66d265be9 100644 --- a/tools/watch.js +++ b/tools/watch.js @@ -191,7 +191,7 @@ WatchSet.fromJSON = function (json) { set.files = _.clone(json.files); var reFromJSON = function (j) { - if (j.$regex) + if (_.has(j, '$regex')) return new RegExp(j.$regex, j.$options); return new RegExp(j); }; From bd05fdb2c5dba32529311369b263d4bbfacf1931 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Fri, 28 Feb 2014 23:12:36 -0800 Subject: [PATCH 15/16] minimongo: support semi-logical $elemMatch ie, a document with some logical operator and some equality in an elemMatch. eg: {a: {$elemMatch: {x: 1, $or: [{a: 1}, {b: 1}]}}} Fixes #1875. --- History.md | 2 ++ packages/minimongo/helpers.js | 10 ++++++++-- packages/minimongo/minimongo_tests.js | 16 ++++++++++++++++ packages/minimongo/selector.js | 2 +- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/History.md b/History.md index f96b92f149..34de130d71 100644 --- a/History.md +++ b/History.md @@ -3,6 +3,8 @@ * Use "faye-websocket" (0.7.2) npm module instead of "websocket" (1.0.8) for server-to-server DDP. +* minimongo: Support {a: {$elemMatch: {x: 1, $or: [{a: 1}, {b: 1}]}}} #1875 + ## v0.7.1.2 diff --git a/packages/minimongo/helpers.js b/packages/minimongo/helpers.js index d4046124f2..ab8cc78b6f 100644 --- a/packages/minimongo/helpers.js +++ b/packages/minimongo/helpers.js @@ -16,7 +16,10 @@ isIndexable = function (x) { return isArray(x) || isPlainObject(x); }; -isOperatorObject = function (valueSelector) { +// Returns true if this is an object with at least one key and all keys begin +// with $. Unless inconsistentOK is set, throws if some keys begin with $ and +// others don't. +isOperatorObject = function (valueSelector, inconsistentOK) { if (!isPlainObject(valueSelector)) return false; @@ -26,7 +29,10 @@ isOperatorObject = function (valueSelector) { if (theseAreOperators === undefined) { theseAreOperators = thisIsOperator; } else if (theseAreOperators !== thisIsOperator) { - throw new Error("Inconsistent operator: " + valueSelector); + if (!inconsistentOK) + throw new Error("Inconsistent operator: " + + JSON.stringify(valueSelector)); + theseAreOperators = false; } }); return !!theseAreOperators; // {} has no operators diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index dd8dc6de94..6c9359761d 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -820,6 +820,12 @@ Tinytest.add("minimongo - selector_compiler", function (test) { nomatch({$or: [{a: 2}, {a: 3}], b: 2}, {a: 1, b: 2}); nomatch({$or: [{a: 1}, {a: 2}], b: 3}, {a: 1, b: 2}); + // Combining $or with equality + match({x: 1, $or: [{a: 1}, {b: 1}]}, {x: 1, b: 1}); + match({$or: [{a: 1}, {b: 1}], x: 1}, {x: 1, b: 1}); + nomatch({x: 1, $or: [{a: 1}, {b: 1}]}, {b: 1}); + nomatch({x: 1, $or: [{a: 1}, {b: 1}]}, {x: 1}); + // $or and $lt, $lte, $gt, $gte match({$or: [{a: {$lte: 1}}, {a: 2}]}, {a: 1}); nomatch({$or: [{a: {$lt: 1}}, {a: 2}]}, {a: 1}); @@ -1103,6 +1109,16 @@ Tinytest.add("minimongo - selector_compiler", function (test) { nomatch({a: {$elemMatch: {x: 5}}}, {a: {x: 5}}); match({a: {$elemMatch: {0: {$gt: 5, $lt: 9}}}}, {a: [[6]]}); match({a: {$elemMatch: {'0.b': {$gt: 5, $lt: 9}}}}, {a: [[{b:6}]]}); + match({a: {$elemMatch: {x: 1, $or: [{a: 1}, {b: 1}]}}}, + {a: [{x: 1, b: 1}]}); + match({a: {$elemMatch: {$or: [{a: 1}, {b: 1}], x: 1}}}, + {a: [{x: 1, b: 1}]}); + nomatch({a: {$elemMatch: {x: 1, $or: [{a: 1}, {b: 1}]}}}, + {a: [{b: 1}]}); + nomatch({a: {$elemMatch: {x: 1, $or: [{a: 1}, {b: 1}]}}}, + {a: [{x: 1}]}); + nomatch({a: {$elemMatch: {x: 1, $or: [{a: 1}, {b: 1}]}}}, + {a: [{x: 1}, {b: 1}]}); // $comment match({a: 5, $comment: "asdf"}, {a: 5}); diff --git a/packages/minimongo/selector.js b/packages/minimongo/selector.js index 3b68a320f9..80636bb12d 100644 --- a/packages/minimongo/selector.js +++ b/packages/minimongo/selector.js @@ -653,7 +653,7 @@ var ELEMENT_OPERATORS = { throw Error("$elemMatch need an object"); var subMatcher, isDocMatcher; - if (isOperatorObject(operand)) { + if (isOperatorObject(operand, true)) { subMatcher = compileValueSelector(operand, matcher); isDocMatcher = false; } else { From 89a96084b864b2bdc64f16a95baa03f4796412bc Mon Sep 17 00:00:00 2001 From: David Glasser Date: Fri, 28 Feb 2014 23:15:33 -0800 Subject: [PATCH 16/16] history update --- History.md | 1 + 1 file changed, 1 insertion(+) diff --git a/History.md b/History.md index 34de130d71..8d0ac759db 100644 --- a/History.md +++ b/History.md @@ -5,6 +5,7 @@ * minimongo: Support {a: {$elemMatch: {x: 1, $or: [{a: 1}, {b: 1}]}}} #1875 +* minimongo: Support {a: {$regex: '', $options: 'i'}} #1874 ## v0.7.1.2