From 78538bc796e18ef3959165c1f3bb1503ab22c0ff Mon Sep 17 00:00:00 2001
From: Lee Byron <lee@leebyron.com>
Date: Tue, 19 Jul 2016 20:47:04 -0700
Subject: [PATCH] [FIX] Throw field errors when failing to coerce Int/Float.

As illustrated by #391, when a value is provided for a field of type Int which cannot be represented by Int (e.g. a 64bit value is provided to the 32bit Int type), then the spec claims a field-error should be raised however currently `null` is returned directly instead.

Spec: https://facebook.github.io/graphql/#sec-Int

This updates to throw meaningful error messages when invalid Int and Float values cannot be coerced without losing information.
---
 src/type/__tests__/serialization-test.js     | 51 +++++++++++++++-----
 src/type/scalars.js                          | 11 ++++-
 src/utilities/__tests__/astFromValue-test.js |  4 +-
 3 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/src/type/__tests__/serialization-test.js b/src/type/__tests__/serialization-test.js
index 4231d073b4..2a8df61bea 100644
--- a/src/type/__tests__/serialization-test.js
+++ b/src/type/__tests__/serialization-test.js
@@ -23,6 +23,9 @@ describe('Type System: Scalar coercion', () => {
     expect(
       GraphQLInt.serialize(1)
     ).to.equal(1);
+    expect(
+      GraphQLInt.serialize('123')
+    ).to.equal(123);
     expect(
       GraphQLInt.serialize(0)
     ).to.equal(0);
@@ -43,25 +46,35 @@ describe('Type System: Scalar coercion', () => {
     ).to.equal(100000);
     // Maybe a safe JavaScript int, but bigger than 2^32, so not
     // representable as a GraphQL Int
-    expect(
+    expect(() =>
       GraphQLInt.serialize(9876504321)
-    ).to.equal(null);
-    expect(
+    ).to.throw(
+      'Int cannot represent non 32-bit signed integer value: 9876504321'
+    );
+    expect(() =>
       GraphQLInt.serialize(-9876504321)
-    ).to.equal(null);
+    ).to.throw(
+      'Int cannot represent non 32-bit signed integer value: -9876504321'
+    );
     // Too big to represent as an Int in JavaScript or GraphQL
-    expect(
+    expect(() =>
       GraphQLInt.serialize(1e100)
-    ).to.equal(null);
-    expect(
+    ).to.throw(
+      'Int cannot represent non 32-bit signed integer value: 1e+100'
+    );
+    expect(() =>
       GraphQLInt.serialize(-1e100)
-    ).to.equal(null);
+    ).to.throw(
+      'Int cannot represent non 32-bit signed integer value: -1e+100'
+    );
     expect(
       GraphQLInt.serialize('-1.1')
     ).to.equal(-1);
-    expect(
+    expect(() =>
       GraphQLInt.serialize('one')
-    ).to.equal(null);
+    ).to.throw(
+      'Int cannot represent non 32-bit signed integer value: one'
+    );
     expect(
       GraphQLInt.serialize(false)
     ).to.equal(0);
@@ -77,6 +90,9 @@ describe('Type System: Scalar coercion', () => {
     expect(
       GraphQLFloat.serialize(0)
     ).to.equal(0.0);
+    expect(
+      GraphQLFloat.serialize('123.5')
+    ).to.equal(123.5);
     expect(
       GraphQLFloat.serialize(-1)
     ).to.equal(-1.0);
@@ -92,15 +108,24 @@ describe('Type System: Scalar coercion', () => {
     expect(
       GraphQLFloat.serialize('-1.1')
     ).to.equal(-1.1);
-    expect(
-      GraphQLFloat.serialize('one')
-    ).to.equal(null);
     expect(
       GraphQLFloat.serialize(false)
     ).to.equal(0.0);
     expect(
       GraphQLFloat.serialize(true)
     ).to.equal(1.0);
+
+    expect(() =>
+      GraphQLFloat.serialize(NaN)
+    ).to.throw(
+      'Float cannot represent non numeric value: NaN'
+    );
+
+    expect(() =>
+      GraphQLFloat.serialize('one')
+    ).to.throw(
+      'Float cannot represent non numeric value: one'
+    );
   });
 
   it('serializes output strings', () => {
diff --git a/src/type/scalars.js b/src/type/scalars.js
index 170a01e8e1..65e2f84af4 100644
--- a/src/type/scalars.js
+++ b/src/type/scalars.js
@@ -24,7 +24,9 @@ function coerceInt(value: mixed): ?number {
   if (num === num && num <= MAX_INT && num >= MIN_INT) {
     return (num < 0 ? Math.ceil : Math.floor)(num);
   }
-  return null;
+  throw new TypeError(
+    'Int cannot represent non 32-bit signed integer value: ' + value
+  );
 }
 
 export const GraphQLInt = new GraphQLScalarType({
@@ -47,7 +49,12 @@ export const GraphQLInt = new GraphQLScalarType({
 
 function coerceFloat(value: mixed): ?number {
   const num = Number(value);
-  return num === num ? num : null;
+  if (num === num) {
+    return num;
+  }
+  throw new TypeError(
+    'Float cannot represent non numeric value: ' + value
+  );
 }
 
 export const GraphQLFloat = new GraphQLScalarType({
diff --git a/src/utilities/__tests__/astFromValue-test.js b/src/utilities/__tests__/astFromValue-test.js
index b2ca525d9c..a1491a7af9 100644
--- a/src/utilities/__tests__/astFromValue-test.js
+++ b/src/utilities/__tests__/astFromValue-test.js
@@ -60,8 +60,8 @@ describe('astFromValue', () => {
     );
 
     // Note: outside the bounds of 32bit signed int.
-    expect(astFromValue(1e40, GraphQLInt)).to.deep.equal(
-      null
+    expect(() => astFromValue(1e40, GraphQLInt)).to.throw(
+      'Int cannot represent non 32-bit signed integer value: 1e+40'
     );
   });