fix: poor error message on scalar deserialization type mismatch (#1187)

* fix: poor error message on scalar deserialization type mismatch When deserializing a scalar (string, boolean, number, ...), and receiving a value that is not a scalar, the error message would not mention waht kind of scalar value was required, making the message less useful than it could be. Used the description of the type reference in the error message so that the specific scalar type is direction mentioned in the error message there. The description of the incorrect value that was received remains unchanged (typically the JSON representation of the value), and may be improved in a subsequent change. * remove unnecessary escape Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
aws · Jan 12, 2020 · fdf8927 · fdf8927
1 parent 71ecca9
commit fdf8927
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 5 deletions.
diff --git a/packages/@jsii/kernel/lib/serialization.ts b/packages/@jsii/kernel/lib/serialization.ts
@@ -137,10 +137,10 @@ export const SERIALIZERS: {[k: string]: Serializer} = {
       const primitiveType = optionalValue.type as spec.PrimitiveTypeReference;
 
       if (!isScalar(value)) {
-        throw new Error(`Expected Scalar, got ${JSON.stringify(value)}`);
+        throw new Error(`Expected ${spec.describeTypeReference(optionalValue.type)}, got ${JSON.stringify(value)}`);
       }
       if (typeof value !== primitiveType.primitive) {
-        throw new Error(`Expected '${primitiveType.primitive}', got ${JSON.stringify(value)} (${typeof value})`);
+        throw new Error(`Expected a ${spec.describeTypeReference(optionalValue.type)}, got ${JSON.stringify(value)} (${typeof value})`);
       }
       return value;
     },
@@ -152,10 +152,10 @@ export const SERIALIZERS: {[k: string]: Serializer} = {
       const primitiveType = optionalValue.type as spec.PrimitiveTypeReference;
 
       if (!isScalar(value)) {
-        throw new Error(`Expected Scalar, got ${JSON.stringify(value)}`);
+        throw new Error(`Expected a ${spec.describeTypeReference(optionalValue.type)}, got ${JSON.stringify(value)}`);
       }
       if (typeof value !== primitiveType.primitive) {
-        throw new Error(`Expected '${primitiveType.primitive}', got ${JSON.stringify(value)} (${typeof value})`);
+        throw new Error(`Expected a ${spec.describeTypeReference(optionalValue.type)}, got ${JSON.stringify(value)} (${typeof value})`);
       }
 
       return value;

diff --git a/packages/@jsii/kernel/package.json b/packages/@jsii/kernel/package.json
@@ -53,7 +53,6 @@
   "jest": {
     "collectCoverage": true,
     "collectCoverageFrom": [
-      "**/bin/**/*.js",
       "**/lib/**/*.js"
     ],
     "coverageReporters": [

diff --git a/packages/@jsii/kernel/test/serialization.test.ts b/packages/@jsii/kernel/test/serialization.test.ts
@@ -0,0 +1,80 @@
+import { OptionalValue, PrimitiveType } from '@jsii/spec';
+import { ObjectTable } from '../lib/objects';
+import { SerializationClass, SerializerHost, SERIALIZERS } from '../lib/serialization';
+
+const TYPE_BOOLEAN: OptionalValue = { type: { primitive: PrimitiveType.Boolean } };
+const TYPE_NUMBER: OptionalValue = { type: { primitive: PrimitiveType.Number } };
+const TYPE_STRING: OptionalValue = { type: { primitive: PrimitiveType.String } };
+const TYPE_VOID = 'void';
+
+describe(SerializationClass.Scalar, () => {
+  const scalarSerializer = SERIALIZERS[SerializationClass.Scalar];
+  const lookupType: SerializerHost['lookupType'] = jest.fn().mockName('host.lookupType');
+  const host: SerializerHost = {
+    debug: jest.fn().mockName('host.debug'),
+    findSymbol: jest.fn().mockName('host.findSymbol'),
+    lookupType,
+    objects: new ObjectTable(lookupType),
+    recurse: jest.fn().mockName('host.recurse'),
+  };
+
+  describe(scalarSerializer.deserialize, () => {
+    describe('void', () => {
+      test('accepts undefined', () =>
+        expect(scalarSerializer.deserialize(
+          undefined,
+          TYPE_VOID,
+          host
+        )).toBeUndefined()
+      );
+
+      test('accepts null', () =>
+        expect(scalarSerializer.deserialize(
+          null,
+          TYPE_VOID,
+          host
+        )).toBeUndefined()
+      );
+
+      test('rejects any value', () =>
+        expect(() => scalarSerializer.deserialize(
+          'nothing!',
+          TYPE_VOID,
+          host
+        )).toThrow(/Encountered unexpected `void` type/)
+      );
+    });
+
+    const scalarTypes = [
+      { name: 'string', type: TYPE_STRING, validValues: ['This is a string!'] },
+      { name: 'number', type: TYPE_NUMBER, validValues: [-1337, 0, Number.EPSILON] },
+      { name: 'boolean', type: TYPE_BOOLEAN, validValues: [true, false] },
+    ];
+    for (const example of scalarTypes) {
+      describe(example.name, () => {
+        test('fails when provided an object', () => {
+          expect(() => scalarSerializer.deserialize(
+            { 'this is not': `a ${example.name}` },
+            example.type,
+            host
+          )).toThrow(`Expected a ${example.name}, got {`);
+        });
+
+        for (const validValue of example.validValues) {
+          test(`accepts: ${validValue}`, () =>
+            expect(scalarSerializer.deserialize(validValue, example.type, host))
+              .toBe(validValue));
+        }
+
+        const invalidValues = scalarTypes.filter(t => t.name !== example.name)
+          .map(t => t.validValues)
+          .reduce((acc, elt) => [...acc, ...elt], new Array<any>());
+        for (const invalidValue of invalidValues) {
+          test(`rejects: ${invalidValue}`, () =>
+            expect(() => scalarSerializer.deserialize(invalidValue, example.type, host))
+              .toThrow(`Expected a ${example.name}, got ${JSON.stringify(invalidValue)} (${typeof invalidValue})`));
+        }
+      });
+    }
+  });
+});