From be0236ca6cedd3c33ca8b1a83f754d2372822f56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20L=C3=B3pez?= Date: Mon, 17 Oct 2016 13:55:10 -0300 Subject: [PATCH 1/5] Add try catch --- src/core/runQuery.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/core/runQuery.ts b/src/core/runQuery.ts index ef5314d24c9..f0f89863101 100644 --- a/src/core/runQuery.ts +++ b/src/core/runQuery.ts @@ -69,11 +69,18 @@ function doRunQuery(options: QueryOptions): Promise { logFunction({action: LogAction.request, step: LogStep.start}); function format(errors: Array): Array { - // TODO: fix types! shouldn't have to cast. - // the blocker is that the typings aren't right atm: - // GraphQLResult returns Array, but the formatError function - // returns Array - return errors.map(options.formatError || formatError as any) as Array; + return errors.map((error) => { + if (options.formatError) { + try { + return options.formatError(error); + } catch (err) { + console.error('Error in formatError function:', err); + return formatError(error); + } + } else { + return formatError(error); + } + }) as Array; } function printStackTrace(error: Error) { From e2403ed06b71396973e02a50ad00849e8da4f24e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20L=C3=B3pez?= Date: Mon, 17 Oct 2016 13:55:15 -0300 Subject: [PATCH 2/5] add tests --- src/integrations/integrations.test.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/integrations/integrations.test.ts b/src/integrations/integrations.test.ts index 863a5627d4f..6c5c9b6b52c 100644 --- a/src/integrations/integrations.test.ts +++ b/src/integrations/integrations.test.ts @@ -424,6 +424,31 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); }); + it('should use default formatter if provided formatError fails', () => { + app = createApp({apolloOptions: { + schema: Schema, + formatError: (err) => { + throw new Error('I should be catched'); + }, + }}); + const logStub = stub(console, 'error'); + const expected = /at resolveOrError/; + app = createApp({apolloOptions: { + schema: Schema, + debug: true, + }}); + const req = request(app) + .post('/graphql') + .send({ + query: 'query test{ testError }', + }); + return req.then((res) => { + logStub.restore(); + expect(logStub.callCount).to.equal(1); + return expect(logStub.getCall(0).args[0]).to.match(expected); + }); + }); + it('sends stack trace to error if debug mode is set', () => { const expected = /at resolveOrError/; const stackTrace = []; From eae2a0ac7e30fc34a28364c14bf8a73082e89fa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20L=C3=B3pez?= Date: Mon, 17 Oct 2016 14:09:21 -0300 Subject: [PATCH 3/5] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c273f5a0e23..110b565aaba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * Pass `ctx` instead of `ctx.request` to options function in Koa integration ([@HriBB](https://github.com/HriBB)) in [PR #154](https://github.com/apollostack/apollo-server/pull/154) * Manage TypeScript declaration files using npm. ([@od1k](https:/github.com/od1k) in [#162](https://github.com/apollostack/apollo-server/pull/162)) * Fix connect example in readme. ([@conrad-vanl](https://github.com/conrad-vanl) in [#165](https://github.com/apollostack/apollo-server/pull/165)) +* Add try/catch to formatError. ([@nicolaslopezj](https://github.com/nicolaslopezj) in [#174](https://github.com/apollostack/apollo-server/pull/174)) ### v0.3.2 * Added missing exports for hapi integration ([@nnance](https://github.com/nnance)) in [PR #152](https://github.com/apollostack/apollo-server/pull/152) From dd5b161662034be6e3f43058c0d5f383f3e0734f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20L=C3=B3pez?= Date: Mon, 17 Oct 2016 16:17:23 -0300 Subject: [PATCH 4/5] Return internal server error --- src/core/runQuery.ts | 3 ++- src/integrations/integrations.test.ts | 12 ++---------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/core/runQuery.ts b/src/core/runQuery.ts index f0f89863101..edad9c031c4 100644 --- a/src/core/runQuery.ts +++ b/src/core/runQuery.ts @@ -75,7 +75,8 @@ function doRunQuery(options: QueryOptions): Promise { return options.formatError(error); } catch (err) { console.error('Error in formatError function:', err); - return formatError(error); + const newError = new Error('Internal server error'); + return formatError(newError); } } else { return formatError(error); diff --git a/src/integrations/integrations.test.ts b/src/integrations/integrations.test.ts index d1ec24973a5..e87c6cee327 100644 --- a/src/integrations/integrations.test.ts +++ b/src/integrations/integrations.test.ts @@ -436,28 +436,20 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); }); - it('should use default formatter if provided formatError fails', () => { + it('Return internal server error when formatError fails', () => { app = createApp({apolloOptions: { schema: Schema, formatError: (err) => { throw new Error('I should be catched'); }, }}); - const logStub = stub(console, 'error'); - const expected = /at resolveOrError/; - app = createApp({apolloOptions: { - schema: Schema, - debug: true, - }}); const req = request(app) .post('/graphql') .send({ query: 'query test{ testError }', }); return req.then((res) => { - logStub.restore(); - expect(logStub.callCount).to.equal(1); - return expect(logStub.getCall(0).args[0]).to.match(expected); + return expect(res.res.body.errors[0].message).to.equal('Internal server error'); }); }); From c1c75d0a5ca708625cf0e1084f0863b7c14d0805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20L=C3=B3pez?= Date: Mon, 17 Oct 2016 16:18:45 -0300 Subject: [PATCH 5/5] change test name --- src/integrations/integrations.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/integrations/integrations.test.ts b/src/integrations/integrations.test.ts index e87c6cee327..8ed6a0b7e93 100644 --- a/src/integrations/integrations.test.ts +++ b/src/integrations/integrations.test.ts @@ -436,7 +436,7 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => { }); }); - it('Return internal server error when formatError fails', () => { + it('sends internal server error when formatError fails', () => { app = createApp({apolloOptions: { schema: Schema, formatError: (err) => {