From b53a0ec447accaf36715e0c520f5a62eee03f690 Mon Sep 17 00:00:00 2001 From: = Date: Thu, 9 May 2019 23:57:52 -0700 Subject: [PATCH 1/6] Changing __indexBuildCompletionCallbackForTests callback to serverStartComplete --- spec/EnableExpressErrorHandler.spec.js | 2 +- spec/ParseLiveQueryServer.spec.js | 4 ++-- spec/ParseServer.spec.js | 2 +- spec/PostgresInitOptions.spec.js | 2 +- spec/helper.js | 2 +- spec/index.spec.js | 4 ++-- src/Options/index.js | 2 +- src/ParseServer.js | 6 ++---- 8 files changed, 11 insertions(+), 13 deletions(-) diff --git a/spec/EnableExpressErrorHandler.spec.js b/spec/EnableExpressErrorHandler.spec.js index 4e520a039a..50fb42dd38 100644 --- a/spec/EnableExpressErrorHandler.spec.js +++ b/spec/EnableExpressErrorHandler.spec.js @@ -17,7 +17,7 @@ describe('Enable express error handler', () => { masterKey: masterKey, serverURL: serverUrl, enableExpressErrorHandler: true, - __indexBuildCompletionCallbackForTests: promise => { + serverStartComplete: promise => { promise.then(() => { expect(Parse.applicationId).toEqual('anOtherTestApp'); const app = express(); diff --git a/spec/ParseLiveQueryServer.spec.js b/spec/ParseLiveQueryServer.spec.js index bda04e814c..f0a5f25df2 100644 --- a/spec/ParseLiveQueryServer.spec.js +++ b/spec/ParseLiveQueryServer.spec.js @@ -158,7 +158,7 @@ describe('ParseLiveQueryServer', function() { classNames: ['Yolo'], }, startLiveQueryServer: true, - __indexBuildCompletionCallbackForTests: promise => { + serverStartComplete: promise => { promise.then(() => { expect(parseServer.liveQueryServer).not.toBeUndefined(); expect(parseServer.liveQueryServer.server).toBe(parseServer.server); @@ -181,7 +181,7 @@ describe('ParseLiveQueryServer', function() { liveQueryServerOptions: { port: 22347, }, - __indexBuildCompletionCallbackForTests: promise => { + serverStartComplete: promise => { promise.then(() => { expect(parseServer.liveQueryServer).not.toBeUndefined(); expect(parseServer.liveQueryServer.server).not.toBe( diff --git a/spec/ParseServer.spec.js b/spec/ParseServer.spec.js index e747e810ce..9cd8afa455 100644 --- a/spec/ParseServer.spec.js +++ b/spec/ParseServer.spec.js @@ -62,7 +62,7 @@ describe('Server Url Checks', () => { } const newConfiguration = Object.assign({}, defaultConfiguration, { databaseAdapter, - __indexBuildCompletionCallbackForTests: promise => { + serverStartComplete: promise => { promise.then(() => { parseServer.handleShutdown(); parseServer.server.close(err => { diff --git a/spec/PostgresInitOptions.spec.js b/spec/PostgresInitOptions.spec.js index 92674f2107..dd1b087a7b 100644 --- a/spec/PostgresInitOptions.spec.js +++ b/spec/PostgresInitOptions.spec.js @@ -28,7 +28,7 @@ function createParseServer(options) { const parseServer = new ParseServer.default( Object.assign({}, defaultConfiguration, options, { serverURL: 'http://localhost:12666/parse', - __indexBuildCompletionCallbackForTests: promise => { + serverStartComplete: promise => { promise.then(() => { expect(Parse.applicationId).toEqual('test'); const app = express(); diff --git a/spec/helper.js b/spec/helper.js index 607f2fcf1d..a058165bb8 100644 --- a/spec/helper.js +++ b/spec/helper.js @@ -150,7 +150,7 @@ const reconfigureServer = changedConfiguration => { defaultConfiguration, changedConfiguration, { - __indexBuildCompletionCallbackForTests: indexBuildPromise => + serverStartComplete: indexBuildPromise => indexBuildPromise.then(() => { resolve(parseServer); }, reject), diff --git a/spec/index.spec.js b/spec/index.spec.js index 6cebb23bc2..5cdbb30e70 100644 --- a/spec/index.spec.js +++ b/spec/index.spec.js @@ -295,7 +295,7 @@ describe('server', () => { appId: 'aTestApp', masterKey: 'aTestMasterKey', serverURL: 'http://localhost:12666/parse', - __indexBuildCompletionCallbackForTests: promise => { + serverStartComplete: promise => { promise.then(() => { expect(Parse.applicationId).toEqual('aTestApp'); const app = express(); @@ -332,7 +332,7 @@ describe('server', () => { appId: 'anOtherTestApp', masterKey: 'anOtherTestMasterKey', serverURL: 'http://localhost:12667/parse', - __indexBuildCompletionCallbackForTests: promise => { + serverStartComplete: promise => { promise .then(() => { expect(Parse.applicationId).toEqual('anOtherTestApp'); diff --git a/src/Options/index.js b/src/Options/index.js index 0fb744cf5c..6cfb5adb02 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -173,7 +173,7 @@ export interface ParseServerOptions { /* Live query server configuration options (will start the liveQuery server) */ liveQueryServerOptions: ?LiveQueryServerOptions; - __indexBuildCompletionCallbackForTests: ?() => void; + serverStartComplete: ?() => void; } export interface CustomPagesOptions { diff --git a/src/ParseServer.js b/src/ParseServer.js index 63e43ab1f3..d6a4964eaa 100644 --- a/src/ParseServer.js +++ b/src/ParseServer.js @@ -80,7 +80,7 @@ class ParseServer { cloud, javascriptKey, serverURL = requiredParameter('You must provide a serverURL!'), - __indexBuildCompletionCallbackForTests = () => {}, + serverStartComplete = () => {}, } = options; // Initialize the node client SDK automatically Parse.initialize(appId, javascriptKey || 'unused', masterKey); @@ -101,9 +101,7 @@ class ParseServer { // Note: Tests will start to fail if any validation happens after this is called. if (process.env.TESTING) { - __indexBuildCompletionCallbackForTests( - Promise.all([dbInitPromise, hooksLoadPromise]) - ); + serverStartComplete(Promise.all([dbInitPromise, hooksLoadPromise])); } if (cloud) { From a44374f98499f045b44ea166c1d4d2e4f90f6c7c Mon Sep 17 00:00:00 2001 From: = Date: Fri, 10 May 2019 00:22:44 -0700 Subject: [PATCH 2/6] Improving serverStartComplete callback to avoid production unhandled promise rejection --- spec/EnableExpressErrorHandler.spec.js | 68 +++++++++++++------------- spec/ParseLiveQueryServer.spec.js | 26 +++++----- spec/ParseServer.spec.js | 18 +++---- spec/PostgresInitOptions.spec.js | 8 +-- spec/helper.js | 9 ++-- spec/index.spec.js | 49 +++++++++---------- src/Options/index.js | 2 +- src/ParseServer.js | 20 ++++++-- 8 files changed, 104 insertions(+), 96 deletions(-) diff --git a/spec/EnableExpressErrorHandler.spec.js b/spec/EnableExpressErrorHandler.spec.js index 50fb42dd38..48b6047092 100644 --- a/spec/EnableExpressErrorHandler.spec.js +++ b/spec/EnableExpressErrorHandler.spec.js @@ -17,47 +17,45 @@ describe('Enable express error handler', () => { masterKey: masterKey, serverURL: serverUrl, enableExpressErrorHandler: true, - serverStartComplete: promise => { - promise.then(() => { - expect(Parse.applicationId).toEqual('anOtherTestApp'); - const app = express(); - app.use('/parse', parseServer); + serverStartComplete: () => { + expect(Parse.applicationId).toEqual('anOtherTestApp'); + const app = express(); + app.use('/parse', parseServer); - server = app.listen(12667); + server = app.listen(12667); - app.use(function(err, req, res, next) { - next; - lastError = err; - }); + app.use(function(err, req, res, next) { + next; + lastError = err; + }); - request({ - method: 'PUT', - url: serverUrl + '/classes/AnyClass/nonExistingId', - headers: { - 'X-Parse-Application-Id': appId, - 'X-Parse-Master-Key': masterKey, - 'Content-Type': 'application/json', - }, - body: { someField: 'blablabla' }, + request({ + method: 'PUT', + url: serverUrl + '/classes/AnyClass/nonExistingId', + headers: { + 'X-Parse-Application-Id': appId, + 'X-Parse-Master-Key': masterKey, + 'Content-Type': 'application/json', + }, + body: { someField: 'blablabla' }, + }) + .then(() => { + fail('Should throw error'); }) - .then(() => { - fail('Should throw error'); - }) - .catch(response => { - const reqError = response.data; - expect(reqError).toBeDefined(); - expect(lastError).toBeDefined(); + .catch(response => { + const reqError = response.data; + expect(reqError).toBeDefined(); + expect(lastError).toBeDefined(); - expect(lastError.code).toEqual(101); - expect(lastError.message).toEqual('Object not found.'); + expect(lastError.code).toEqual(101); + expect(lastError.message).toEqual('Object not found.'); - expect(lastError.code).toEqual(reqError.code); - expect(lastError.message).toEqual(reqError.error); - }) - .then(() => { - server.close(done); - }); - }); + expect(lastError.code).toEqual(reqError.code); + expect(lastError.message).toEqual(reqError.error); + }) + .then(() => { + server.close(done); + }); }, }) ); diff --git a/spec/ParseLiveQueryServer.spec.js b/spec/ParseLiveQueryServer.spec.js index f0a5f25df2..89c10d3f74 100644 --- a/spec/ParseLiveQueryServer.spec.js +++ b/spec/ParseLiveQueryServer.spec.js @@ -158,12 +158,10 @@ describe('ParseLiveQueryServer', function() { classNames: ['Yolo'], }, startLiveQueryServer: true, - serverStartComplete: promise => { - promise.then(() => { - expect(parseServer.liveQueryServer).not.toBeUndefined(); - expect(parseServer.liveQueryServer.server).toBe(parseServer.server); - parseServer.server.close(() => done()); - }); + serverStartComplete: () => { + expect(parseServer.liveQueryServer).not.toBeUndefined(); + expect(parseServer.liveQueryServer.server).toBe(parseServer.server); + parseServer.server.close(() => done()); }, }); }); @@ -181,15 +179,13 @@ describe('ParseLiveQueryServer', function() { liveQueryServerOptions: { port: 22347, }, - serverStartComplete: promise => { - promise.then(() => { - expect(parseServer.liveQueryServer).not.toBeUndefined(); - expect(parseServer.liveQueryServer.server).not.toBe( - parseServer.server - ); - parseServer.liveQueryServer.server.close(() => { - parseServer.server.close(() => done()); - }); + serverStartComplete: () => { + expect(parseServer.liveQueryServer).not.toBeUndefined(); + expect(parseServer.liveQueryServer.server).not.toBe( + parseServer.server + ); + parseServer.liveQueryServer.server.close(() => { + parseServer.server.close(() => done()); }); }, }); diff --git a/spec/ParseServer.spec.js b/spec/ParseServer.spec.js index 9cd8afa455..d6206b340c 100644 --- a/spec/ParseServer.spec.js +++ b/spec/ParseServer.spec.js @@ -62,16 +62,14 @@ describe('Server Url Checks', () => { } const newConfiguration = Object.assign({}, defaultConfiguration, { databaseAdapter, - serverStartComplete: promise => { - promise.then(() => { - parseServer.handleShutdown(); - parseServer.server.close(err => { - if (err) { - done.fail('Close Server Error'); - } - reconfigureServer({}).then(() => { - done(); - }); + serverStartComplete: () => { + parseServer.handleShutdown(); + parseServer.server.close(err => { + if (err) { + done.fail('Close Server Error'); + } + reconfigureServer({}).then(() => { + done(); }); }); }, diff --git a/spec/PostgresInitOptions.spec.js b/spec/PostgresInitOptions.spec.js index dd1b087a7b..956d8e543a 100644 --- a/spec/PostgresInitOptions.spec.js +++ b/spec/PostgresInitOptions.spec.js @@ -28,8 +28,10 @@ function createParseServer(options) { const parseServer = new ParseServer.default( Object.assign({}, defaultConfiguration, options, { serverURL: 'http://localhost:12666/parse', - serverStartComplete: promise => { - promise.then(() => { + serverStartComplete: error => { + if (error) { + reject(error); + } else { expect(Parse.applicationId).toEqual('test'); const app = express(); app.use('/parse', parseServer.app); @@ -37,7 +39,7 @@ function createParseServer(options) { const server = app.listen(12666); Parse.serverURL = 'http://localhost:12666/parse'; resolve(server); - }, reject); + } }, }) ); diff --git a/spec/helper.js b/spec/helper.js index a058165bb8..844ca6777a 100644 --- a/spec/helper.js +++ b/spec/helper.js @@ -150,10 +150,13 @@ const reconfigureServer = changedConfiguration => { defaultConfiguration, changedConfiguration, { - serverStartComplete: indexBuildPromise => - indexBuildPromise.then(() => { + serverStartComplete: error => { + if (error) { + reject(error); + } else { resolve(parseServer); - }, reject), + } + }, mountPath: '/1', port, } diff --git a/spec/index.spec.js b/spec/index.spec.js index 5cdbb30e70..533a734b8f 100644 --- a/spec/index.spec.js +++ b/spec/index.spec.js @@ -295,30 +295,28 @@ describe('server', () => { appId: 'aTestApp', masterKey: 'aTestMasterKey', serverURL: 'http://localhost:12666/parse', - serverStartComplete: promise => { - promise.then(() => { - expect(Parse.applicationId).toEqual('aTestApp'); - const app = express(); - app.use('/parse', parseServer.app); - - const server = app.listen(12666); - const obj = new Parse.Object('AnObject'); - let objId; - obj - .save() - .then(obj => { - objId = obj.id; - const q = new Parse.Query('AnObject'); - return q.first(); - }) - .then(obj => { - expect(obj.id).toEqual(objId); - server.close(done); - }) - .catch(() => { - server.close(done); - }); - }); + serverStartComplete: () => { + expect(Parse.applicationId).toEqual('aTestApp'); + const app = express(); + app.use('/parse', parseServer.app); + + const server = app.listen(12666); + const obj = new Parse.Object('AnObject'); + let objId; + obj + .save() + .then(obj => { + objId = obj.id; + const q = new Parse.Query('AnObject'); + return q.first(); + }) + .then(obj => { + expect(obj.id).toEqual(objId); + server.close(done); + }) + .catch(() => { + server.close(done); + }); }, }) ); @@ -332,7 +330,8 @@ describe('server', () => { appId: 'anOtherTestApp', masterKey: 'anOtherTestMasterKey', serverURL: 'http://localhost:12667/parse', - serverStartComplete: promise => { + serverStartComplete: error => { + const promise = error ? Promise.reject(error) : Promise.resolve(); promise .then(() => { expect(Parse.applicationId).toEqual('anOtherTestApp'); diff --git a/src/Options/index.js b/src/Options/index.js index 6cfb5adb02..b7cfd22ab6 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -173,7 +173,7 @@ export interface ParseServerOptions { /* Live query server configuration options (will start the liveQuery server) */ liveQueryServerOptions: ?LiveQueryServerOptions; - serverStartComplete: ?() => void; + serverStartComplete: ?(error: ?Error) => void; } export interface CustomPagesOptions { diff --git a/src/ParseServer.js b/src/ParseServer.js index d6a4964eaa..47e488d851 100644 --- a/src/ParseServer.js +++ b/src/ParseServer.js @@ -80,7 +80,7 @@ class ParseServer { cloud, javascriptKey, serverURL = requiredParameter('You must provide a serverURL!'), - serverStartComplete = () => {}, + serverStartComplete, } = options; // Initialize the node client SDK automatically Parse.initialize(appId, javascriptKey || 'unused', masterKey); @@ -100,9 +100,21 @@ class ParseServer { const hooksLoadPromise = hooksController.load(); // Note: Tests will start to fail if any validation happens after this is called. - if (process.env.TESTING) { - serverStartComplete(Promise.all([dbInitPromise, hooksLoadPromise])); - } + Promise.all([dbInitPromise, hooksLoadPromise]) + .then(() => { + if (serverStartComplete) { + serverStartComplete(); + } + }) + .catch(error => { + if (serverStartComplete) { + serverStartComplete(error); + } else { + // eslint-disable-next-line no-console + console.error(error); + process.exit(1); + } + }); if (cloud) { addParseCloud(); From d04e91cbf4096a0d227454ba35236da60d4299ea Mon Sep 17 00:00:00 2001 From: = Date: Fri, 10 May 2019 00:43:16 -0700 Subject: [PATCH 3/6] Add test to check inexistence of unhandled promise rejection on server fail --- spec/ParseServer.spec.js | 22 ++++++++++++++++++++++ spec/support/FailingServer.js | 10 ++++++++++ 2 files changed, 32 insertions(+) create mode 100755 spec/support/FailingServer.js diff --git a/spec/ParseServer.spec.js b/spec/ParseServer.spec.js index d6206b340c..5477e0dcb4 100644 --- a/spec/ParseServer.spec.js +++ b/spec/ParseServer.spec.js @@ -6,6 +6,8 @@ const MongoStorageAdapter = require('../lib/Adapters/Storage/Mongo/MongoStorageA const PostgresStorageAdapter = require('../lib/Adapters/Storage/Postgres/PostgresStorageAdapter') .default; const ParseServer = require('../lib/ParseServer').default; +const path = require('path'); +const { spawn } = require('child_process'); describe('Server Url Checks', () => { let server; @@ -76,4 +78,24 @@ describe('Server Url Checks', () => { }); const parseServer = ParseServer.start(newConfiguration); }); + + it('does not have unhandled promise rejection in the case of load error', done => { + const parseServerProcess = spawn( + path.resolve(__dirname, './support/FailingServer.js') + ); + let stdout; + let stderr; + parseServerProcess.stdout.on('data', data => { + stdout = data.toString(); + }); + parseServerProcess.stderr.on('data', data => { + stderr = data.toString(); + }); + parseServerProcess.on('close', code => { + expect(code).toEqual(1); + expect(stdout).toBeUndefined(); + expect(stderr).toContain('MongoNetworkError:'); + done(); + }); + }); }); diff --git a/spec/support/FailingServer.js b/spec/support/FailingServer.js new file mode 100755 index 0000000000..f5ca8743d6 --- /dev/null +++ b/spec/support/FailingServer.js @@ -0,0 +1,10 @@ +#!/usr/bin/env node + +const ParseServer = require('../../lib/index').ParseServer; + +ParseServer.start({ + appId: 'test', + masterKey: 'test', + databaseURI: + 'mongodb://doesnotexist:27017/parseServerMongoAdapterTestDatabase', +}); From 4f8e08a98a6b0850d651cbc05c82bb4cc17a7ead Mon Sep 17 00:00:00 2001 From: = Date: Fri, 10 May 2019 01:20:37 -0700 Subject: [PATCH 4/6] Removing some hooks delays --- spec/CloudCode.spec.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index af9de4efef..2113f8b44d 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -243,13 +243,13 @@ describe('Cloud Code', () => { Parse.Cloud.afterSave('AfterSaveTest', function(req) { const obj = new Parse.Object('AfterSaveProof'); obj.set('proof', req.object.id); - obj.save(); + obj.save().then(test); }); const obj = new Parse.Object('AfterSaveTest'); obj.save(); - setTimeout(function() { + function test() { const query = new Parse.Query('AfterSaveProof'); query.equalTo('proof', obj.id); query.find().then( @@ -262,7 +262,7 @@ describe('Cloud Code', () => { done(); } ); - }, 500); + } }); it('test afterSave ran on created object and returned a promise', function(done) { @@ -519,7 +519,7 @@ describe('Cloud Code', () => { Parse.Cloud.afterDelete('AfterDeleteTest', function(req) { const obj = new Parse.Object('AfterDeleteProof'); obj.set('proof', req.object.id); - obj.save(); + obj.save().then(test); }); const obj = new Parse.Object('AfterDeleteTest'); @@ -527,7 +527,7 @@ describe('Cloud Code', () => { obj.destroy(); }); - setTimeout(function() { + function test() { const query = new Parse.Query('AfterDeleteProof'); query.equalTo('proof', obj.id); query.find().then( @@ -540,7 +540,7 @@ describe('Cloud Code', () => { done(); } ); - }, 500); + } }); it('test cloud function return types', function(done) { From 35d167f34741106b011e1f58cae8ca7bc70999aa Mon Sep 17 00:00:00 2001 From: = Date: Fri, 10 May 2019 01:28:40 -0700 Subject: [PATCH 5/6] Removing delay after reconfigureServer --- spec/FilesController.spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/FilesController.spec.js b/spec/FilesController.spec.js index 0cfcf7e37e..48908c6df3 100644 --- a/spec/FilesController.spec.js +++ b/spec/FilesController.spec.js @@ -46,7 +46,6 @@ describe('FilesController', () => { const logController = new LoggerController(new WinstonLoggerAdapter()); reconfigureServer({ filesAdapter: mockAdapter }) - .then(() => new Promise(resolve => setTimeout(resolve, 1000))) .then(() => new Parse.File('yolo.txt', [1, 2, 3], 'text/plain').save()) .then( () => done.fail('should not succeed'), From b40d0404383c3d0e48f67b2ffbc05d6d42a7dddd Mon Sep 17 00:00:00 2001 From: = Date: Sun, 12 May 2019 23:50:23 -0700 Subject: [PATCH 6/6] Improving code style --- spec/ParseLiveQueryServer.spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/ParseLiveQueryServer.spec.js b/spec/ParseLiveQueryServer.spec.js index 89c10d3f74..f5b5469a23 100644 --- a/spec/ParseLiveQueryServer.spec.js +++ b/spec/ParseLiveQueryServer.spec.js @@ -161,7 +161,7 @@ describe('ParseLiveQueryServer', function() { serverStartComplete: () => { expect(parseServer.liveQueryServer).not.toBeUndefined(); expect(parseServer.liveQueryServer.server).toBe(parseServer.server); - parseServer.server.close(() => done()); + parseServer.server.close(done); }, }); }); @@ -184,9 +184,9 @@ describe('ParseLiveQueryServer', function() { expect(parseServer.liveQueryServer.server).not.toBe( parseServer.server ); - parseServer.liveQueryServer.server.close(() => { - parseServer.server.close(() => done()); - }); + parseServer.liveQueryServer.server.close( + parseServer.server.close.bind(parseServer.server, done) + ); }, }); });