Fixing the GRPC toError() bug with ErrorValidation

emmacasolin · emmacasolin · commit d05d4e5da51f · 2022-02-14T14:47:43.000+11:00
diff --git a/src/validation/errors.ts b/src/validation/errors.ts
@@ -2,24 +2,45 @@ import { CustomError } from 'ts-custom-error';
 import { ErrorPolykey, sysexits } from '../errors';
 
 /**
- * This packages the `ErrorParse` array into the `data` property
- * This is to allow encoding to and decoding from GRPC errors
+ * Generic error containing all parsing errors that occurred during
+ * execution.
  */
 class ErrorValidation extends ErrorPolykey {
-  public readonly errors: Array<ErrorParse>;
-  constructor(errors: Array<ErrorParse>) {
+  description = 'Input data failed validation';
+  exitCode = sysexits.DATAERR;
+  public errors: Array<ErrorParse>;
+  constructor(message, data) {
+    super(message, data);
+    if (data.errors != null) {
+      const errors: Array<ErrorParse> = [];
+      for (const eData of data.errors) {
+        const errorParse = new ErrorParse(eData.message);
+        errorParse.keyPath = eData.keyPath;
+        errorParse.value = eData.value;
+        errorParse.context = eData.context;
+        errors.push(errorParse);
+      }
+      this.errors = errors;
+    }
+  }
+
+  /**
+   * This packages an `ErrorParse` array into the `data` property
+   * This is to allow encoding to and decoding from GRPC errors
+   */
+  static createFromErrors(errors: Array<ErrorParse>): ErrorValidation {
     const message = errors.map((e) => e.message).join('; ');
     const data = {
       errors: errors.map((e) => ({
         message: e.message,
         keyPath: e.keyPath,
         value: e.value.valueOf(),
+        context: e.context,
       })),
     };
-    super(message, data);
-    this.description = 'Input data failed validation';
-    this.exitCode = sysexits.DATAERR;
-    this.errors = errors;
+    const e = new ErrorValidation(message, data);
+    e.errors = errors;
+    return e;
   }
 }
 
diff --git a/src/validation/index.ts b/src/validation/index.ts
@@ -41,13 +41,13 @@ async function validate(
     data = await parse_([], data, { undefined: data });
   } catch (e) {
     if (e instanceof validationErrors.ErrorParse) {
-      throw new validationErrors.ErrorValidation(errors);
+      throw validationErrors.ErrorValidation.createFromErrors(errors);
     } else {
       throw e;
     }
   }
   if (errors.length > 0) {
-    throw new validationErrors.ErrorValidation(errors);
+    throw validationErrors.ErrorValidation.createFromErrors(errors);
   }
   return data;
 }
@@ -88,13 +88,13 @@ function validateSync(
     data = parse_([], data, { undefined: data });
   } catch (e) {
     if (e instanceof validationErrors.ErrorParse) {
-      throw new validationErrors.ErrorValidation(errors);
+      throw validationErrors.ErrorValidation.createFromErrors(errors);
     } else {
       throw e;
     }
   }
   if (errors.length > 0) {
-    throw new validationErrors.ErrorValidation(errors);
+    throw validationErrors.ErrorValidation.createFromErrors(errors);
   }
   return data;
 }
diff --git a/tests/client/service/identitiesAuthenticate.test.ts b/tests/client/service/identitiesAuthenticate.test.ts
@@ -16,6 +16,7 @@ import {
 import identitiesAuthenticate from '@/client/service/identitiesAuthenticate';
 import * as identitiesPB from '@/proto/js/polykey/v1/identities/identities_pb';
 import { utils as nodesUtils } from '@/nodes';
+import * as validationErrors from '@/validation/errors';
 import TestProvider from '../../identities/TestProvider';
 
 describe('identitiesAuthenticate', () => {
@@ -87,7 +88,6 @@ describe('identitiesAuthenticate', () => {
   test('authenticates identity', async () => {
     const request = new identitiesPB.Provider();
     request.setProviderId(testToken.providerId);
-    request.setIdentityId(testToken.identityId);
     const response = grpcClient.identitiesAuthenticate(
       request,
       clientUtils.encodeAuthFromPassword(password),
@@ -123,4 +123,16 @@ describe('identitiesAuthenticate', () => {
       testToken.identityId,
     );
   });
+  test('cannot authenticate invalid provider', async () => {
+    const request = new identitiesPB.Provider();
+    request.setProviderId('');
+    await expect(
+      grpcClient
+        .identitiesAuthenticate(
+          request,
+          clientUtils.encodeAuthFromPassword(password),
+        )
+        .next(),
+    ).rejects.toThrow(validationErrors.ErrorValidation);
+  });
 });
diff --git a/tests/client/service/identitiesClaim.test.ts b/tests/client/service/identitiesClaim.test.ts
@@ -22,6 +22,7 @@ import {
 import identitiesClaim from '@/client/service/identitiesClaim';
 import * as identitiesPB from '@/proto/js/polykey/v1/identities/identities_pb';
 import * as claimsUtils from '@/claims/utils';
+import * as validationErrors from '@/validation/errors';
 import TestProvider from '../../identities/TestProvider';
 import * as testUtils from '../../utils';
 
@@ -199,4 +200,31 @@ describe('identitiesClaim', () => {
       testToken.identityId,
     );
   });
+  test('cannot claim invalid identity', async () => {
+    const request = new identitiesPB.Provider();
+    request.setIdentityId('');
+    request.setProviderId(testToken.providerId);
+    await expect(
+      grpcClient.identitiesClaim(
+        request,
+        clientUtils.encodeAuthFromPassword(password),
+      ),
+    ).rejects.toThrow(validationErrors.ErrorValidation);
+    request.setIdentityId(testToken.identityId);
+    request.setProviderId('');
+    await expect(
+      grpcClient.identitiesClaim(
+        request,
+        clientUtils.encodeAuthFromPassword(password),
+      ),
+    ).rejects.toThrow(validationErrors.ErrorValidation);
+    request.setIdentityId('');
+    request.setProviderId('');
+    await expect(
+      grpcClient.identitiesClaim(
+        request,
+        clientUtils.encodeAuthFromPassword(password),
+      ),
+    ).rejects.toThrow(validationErrors.ErrorValidation);
+  });
 });
diff --git a/tests/client/service/nodesAdd.test.ts b/tests/client/service/nodesAdd.test.ts
@@ -19,6 +19,7 @@ import nodesAdd from '@/client/service/nodesAdd';
 import * as nodesPB from '@/proto/js/polykey/v1/nodes/nodes_pb';
 import * as utilsPB from '@/proto/js/polykey/v1/utils/utils_pb';
 import { utils as nodesUtils } from '@/nodes';
+import * as validationErrors from '@/validation/errors';
 import * as testUtils from '../../utils';
 
 describe('nodesAdd', () => {
@@ -155,4 +156,37 @@ describe('nodesAdd', () => {
     expect(result!.host).toBe('127.0.0.1');
     expect(result!.port).toBe(11111);
   });
+  test('cannot add invalid node', async () => {
+    // Invalid host
+    const addressMessage = new nodesPB.Address();
+    addressMessage.setHost('');
+    addressMessage.setPort(11111);
+    const request = new nodesPB.NodeAddress();
+    request.setNodeId('vrsc24a1er424epq77dtoveo93meij0pc8ig4uvs9jbeld78n9nl0');
+    request.setAddress(addressMessage);
+    await expect(
+      grpcClient.nodesAdd(
+        request,
+        clientUtils.encodeAuthFromPassword(password),
+      ),
+    ).rejects.toThrow(validationErrors.ErrorValidation);
+    // Invalid port
+    addressMessage.setHost('127.0.0.1');
+    addressMessage.setPort(111111);
+    await expect(
+      grpcClient.nodesAdd(
+        request,
+        clientUtils.encodeAuthFromPassword(password),
+      ),
+    ).rejects.toThrow(validationErrors.ErrorValidation);
+    // Invalid nodeid
+    addressMessage.setPort(11111);
+    request.setNodeId('nodeId');
+    await expect(
+      grpcClient.nodesAdd(
+        request,
+        clientUtils.encodeAuthFromPassword(password),
+      ),
+    ).rejects.toThrow(validationErrors.ErrorValidation);
+  });
 });
diff --git a/tests/client/service/nodesClaim.test.ts b/tests/client/service/nodesClaim.test.ts
@@ -22,6 +22,7 @@ import {
 import nodesClaim from '@/client/service/nodesClaim';
 import * as nodesPB from '@/proto/js/polykey/v1/nodes/nodes_pb';
 import * as utilsPB from '@/proto/js/polykey/v1/utils/utils_pb';
+import * as validationErrors from '@/validation/errors';
 import * as testUtils from '../../utils';
 
 describe('nodesClaim', () => {
@@ -212,4 +213,14 @@ describe('nodesClaim', () => {
     expect(response).toBeInstanceOf(utilsPB.StatusMessage);
     expect(response.getSuccess()).toBeTruthy();
   });
+  test('cannot claim an invalid node', async () => {
+    const request = new nodesPB.Claim();
+    request.setNodeId('nodeId');
+    await expect(
+      grpcClient.nodesClaim(
+        request,
+        clientUtils.encodeAuthFromPassword(password),
+      ),
+    ).rejects.toThrow(validationErrors.ErrorValidation);
+  });
 });
diff --git a/tests/client/service/nodesFind.test.ts b/tests/client/service/nodesFind.test.ts
@@ -17,6 +17,7 @@ import {
 } from '@/client';
 import nodesFind from '@/client/service/nodesFind';
 import * as nodesPB from '@/proto/js/polykey/v1/nodes/nodes_pb';
+import * as validationErrors from '@/validation/errors';
 import * as testUtils from '../../utils';
 
 describe('nodesFind', () => {
@@ -153,4 +154,14 @@ describe('nodesFind', () => {
     expect(response.getAddress()!.getHost()).toBe('127.0.0.1');
     expect(response.getAddress()!.getPort()).toBe(11111);
   });
+  test('cannot find an invalid node', async () => {
+    const request = new nodesPB.Node();
+    request.setNodeId('nodeId');
+    await expect(
+      grpcClient.nodesFind(
+        request,
+        clientUtils.encodeAuthFromPassword(password),
+      ),
+    ).rejects.toThrow(validationErrors.ErrorValidation);
+  });
 });
diff --git a/tests/client/service/nodesPing.test.ts b/tests/client/service/nodesPing.test.ts
@@ -18,6 +18,7 @@ import {
 import nodesPing from '@/client/service/nodesPing';
 import * as nodesPB from '@/proto/js/polykey/v1/nodes/nodes_pb';
 import * as utilsPB from '@/proto/js/polykey/v1/utils/utils_pb';
+import * as validationErrors from '@/validation/errors';
 import * as testUtils from '../../utils';
 
 describe('nodesPing', () => {
@@ -158,4 +159,14 @@ describe('nodesPing', () => {
     expect(response).toBeInstanceOf(utilsPB.StatusMessage);
     expect(response.getSuccess()).toBeTruthy();
   });
+  test('cannot ping an invalid node', async () => {
+    const request = new nodesPB.Node();
+    request.setNodeId('nodeId');
+    await expect(
+      grpcClient.nodesPing(
+        request,
+        clientUtils.encodeAuthFromPassword(password),
+      ),
+    ).rejects.toThrow(validationErrors.ErrorValidation);
+  });
 });

Original file line number	Diff line number	Diff line change
`@@ -41,13 +41,13 @@ async function validate(`
`41`	`41`	`data = await parse_([], data, { undefined: data });`
`42`	`42`	`} catch (e) {`
`43`	`43`	`if (e instanceof validationErrors.ErrorParse) {`
`44`		`- throw new validationErrors.ErrorValidation(errors);`
	`44`	`+ throw validationErrors.ErrorValidation.createFromErrors(errors);`
`45`	`45`	`} else {`
`46`	`46`	`throw e;`
`47`	`47`	`}`
`48`	`48`	`}`
`49`	`49`	`if (errors.length > 0) {`
`50`		`- throw new validationErrors.ErrorValidation(errors);`
	`50`	`+ throw validationErrors.ErrorValidation.createFromErrors(errors);`
`51`	`51`	`}`
`52`	`52`	`return data;`
`53`	`53`	`}`
`@@ -88,13 +88,13 @@ function validateSync(`
`88`	`88`	`data = parse_([], data, { undefined: data });`
`89`	`89`	`} catch (e) {`
`90`	`90`	`if (e instanceof validationErrors.ErrorParse) {`
`91`		`- throw new validationErrors.ErrorValidation(errors);`
	`91`	`+ throw validationErrors.ErrorValidation.createFromErrors(errors);`
`92`	`92`	`} else {`
`93`	`93`	`throw e;`
`94`	`94`	`}`
`95`	`95`	`}`
`96`	`96`	`if (errors.length > 0) {`
`97`		`- throw new validationErrors.ErrorValidation(errors);`
	`97`	`+ throw validationErrors.ErrorValidation.createFromErrors(errors);`
`98`	`98`	`}`
`99`	`99`	`return data;`
`100`	`100`	`}`