diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index d35b80d279..5dfb6dad4b 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -2345,6 +2345,27 @@ describe('cloud functions', () => { }); describe('beforeSave hooks', () => { + it('should reject with the trigger error when the object cannot be serialized for logging', async () => { + Parse.Cloud.beforeSave('MyObject', req => { + const circular = {}; + circular.self = circular; + req.object.set('circular', circular); + throw new Parse.Error(Parse.Error.SCRIPT_FAILED, 'blocked'); + }); + await expectAsync(new Parse.Object('MyObject').save()).toBeRejectedWith( + new Parse.Error(Parse.Error.SCRIPT_FAILED, 'blocked') + ); + }); + + it('should settle when a successful trigger leaves an object that cannot be serialized', async () => { + Parse.Cloud.beforeSave('MyObject', req => { + const circular = {}; + circular.self = circular; + req.object.set('circular', circular); + }); + await expectAsync(new Parse.Object('MyObject').save()).toBeRejected(); + }); + it('should have request headers', done => { Parse.Cloud.beforeSave('MyObject', req => { expect(req.headers).toBeDefined(); @@ -2977,6 +2998,44 @@ describe('beforeFind hooks', () => { }); describe('afterFind hooks', () => { + const invalidAfterFindError = new Parse.Error( + Parse.Error.SCRIPT_FAILED, + 'afterFind trigger must return an array of objects.' + ); + + for (const [type, value] of [ + ['an object', { foo: 'bar' }], + ['a string', 'invalid'], + ['a number', 1], + ]) { + it(`should reject a find when afterFind returns ${type}`, async () => { + Parse.Cloud.afterFind('MyObject', () => value); + await new Parse.Object('MyObject').save(); + await expectAsync(new Parse.Query('MyObject').find()).toBeRejectedWith( + invalidAfterFindError + ); + }); + } + + it('should reject a get when afterFind returns a non-array', async () => { + Parse.Cloud.afterFind('MyObject', () => ({ foo: 'bar' })); + const obj = await new Parse.Object('MyObject').save(); + await expectAsync(new Parse.Query('MyObject').get(obj.id)).toBeRejectedWith( + invalidAfterFindError + ); + }); + + for (const value of [null, undefined]) { + it(`should return the original objects when afterFind returns ${value}`, async () => { + Parse.Cloud.afterFind('MyObject', () => value); + const obj = await new Parse.Object('MyObject').save({ key: 'value' }); + const results = await new Parse.Query('MyObject').find(); + expect(results.length).toBe(1); + expect(results[0].id).toBe(obj.id); + expect(results[0].get('key')).toBe('value'); + }); + } + it('should add afterFind trigger', done => { Parse.Cloud.afterFind('MyObject', req => { const q = req.query; diff --git a/src/triggers.js b/src/triggers.js index f34cfcc7ec..1c7336400d 100644 --- a/src/triggers.js +++ b/src/triggers.js @@ -379,6 +379,14 @@ export function getResponseObject(request, resolve, reject) { if (!response) { response = request.objects; } + if (!Array.isArray(response)) { + return reject( + new Parse.Error( + Parse.Error.SCRIPT_FAILED, + 'afterFind trigger must return an array of objects.' + ) + ); + } response = response.map(object => { return toJSONwithObjects(object); }); @@ -455,6 +463,16 @@ function logTriggerSuccessBeforeHook(triggerType, className, input, result, auth ); } +// Logging must not prevent a trigger promise from settling, e.g. when the +// object cannot be serialized because a trigger set a circular attribute. +function logSafely(log) { + try { + log(); + } catch (e) { + logger.error('Failed to log trigger result', { error: e }); + } +} + function logTriggerErrorBeforeHook(triggerType, className, input, auth, error, logLevel) { if (logLevel === 'silent') { return; @@ -553,7 +571,8 @@ export function maybeRunAfterFindTrigger( } return responseFromTrigger; }) - .then(success, error); + .then(success, error) + .catch(error); }).then(resultsAsJSON => { logTriggerAfterHook( triggerType, @@ -950,15 +969,17 @@ export function maybeRunTrigger( var { success, error } = getResponseObject( request, object => { - logTriggerSuccessBeforeHook( - triggerType, - parseObject.className, - parseObject.toJSON(), - object, - auth, - triggerType.startsWith('after') - ? config.logLevels.triggerAfter - : config.logLevels.triggerBeforeSuccess + logSafely(() => + logTriggerSuccessBeforeHook( + triggerType, + parseObject.className, + parseObject.toJSON(), + object, + auth, + triggerType.startsWith('after') + ? config.logLevels.triggerAfter + : config.logLevels.triggerBeforeSuccess + ) ); if ( triggerType === Types.beforeSave || @@ -971,13 +992,15 @@ export function maybeRunTrigger( resolve(object); }, error => { - logTriggerErrorBeforeHook( - triggerType, - parseObject.className, - parseObject.toJSON(), - auth, - error, - config.logLevels.triggerBeforeError + logSafely(() => + logTriggerErrorBeforeHook( + triggerType, + parseObject.className, + parseObject.toJSON(), + auth, + error, + config.logLevels.triggerBeforeError + ) ); reject(error); } @@ -1026,7 +1049,8 @@ export function maybeRunTrigger( return promise; }) - .then(success, error); + .then(success, error) + .catch(error); }); }