feat(permission-controller): Re-throw all well-formed errors (#4172)

When re-throwing validation errors in `requestPermissions()`, include
original message from all well-formed errors. Also restore 100% unit
test coverage.

packages/permission-controller/jest.config.js

@@ -17,10 +17,10 @@ module.exports = merge(baseConfig, {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 98.8,
+      branches: 100,
       functions: 100,
-      lines: 99.78,
-      statements: 99.78,
+      lines: 100,
+      statements: 100,
     },
   },
 });

packages/permission-controller/src/PermissionController.test.ts

@@ -3804,7 +3804,7 @@ describe('PermissionController', () => {
       expect(callActionSpy).not.toHaveBeenCalled();
     });
 
-    it('throws if subjectTypes do not match', async () => {
+    it('throws if subjectTypes do not match (restricted method)', async () => {
       const options = getPermissionControllerOptions();
       const { messenger } = options;
       const origin = 'metamask.io';
@@ -3840,6 +3840,42 @@ describe('PermissionController', () => {
       );
     });
 
+    it('throws if subjectTypes do not match (endowment)', async () => {
+      const options = getPermissionControllerOptions();
+      const { messenger } = options;
+      const origin = 'metamask.io';
+
+      const callActionSpy = jest
+        .spyOn(messenger, 'call')
+        .mockImplementationOnce(() => {
+          return {
+            origin,
+            name: origin,
+            subjectType: SubjectType.Website,
+            iconUrl: null,
+            extensionId: null,
+          };
+        });
+
+      const controller = getDefaultPermissionController(options);
+      await expect(
+        controller.requestPermissions(
+          { origin },
+          {
+            [PermissionNames.endowmentSnapsOnly]: {},
+          },
+        ),
+      ).rejects.toThrow(
+        'Subject "metamask.io" has no permission for "endowmentSnapsOnly".',
+      );
+
+      expect(callActionSpy).toHaveBeenCalledTimes(1);
+      expect(callActionSpy).toHaveBeenCalledWith(
+        'SubjectMetadataController:getSubjectMetadata',
+        origin,
+      );
+    });
+
     it('does not throw if subjectTypes match', async () => {
       const options = getPermissionControllerOptions();
       const { messenger } = options;
@@ -3929,7 +3965,7 @@ describe('PermissionController', () => {
       );
     });
 
-    it('throws if the "caveat" property of a requested permission is invalid', async () => {
+    it('throws if the "caveats" property of a requested permission is invalid', async () => {
       const options = getPermissionControllerOptions();
       const { messenger } = options;
       const origin = 'metamask.io';
@@ -4334,6 +4370,67 @@ describe('PermissionController', () => {
         true,
       );
     });
+
+    it('correctly throws errors that do not inherit from JsonRpcError', async () => {
+      const options = getPermissionControllerOptions();
+      const { messenger } = options;
+      const origin = 'metamask.io';
+      const controller = getDefaultPermissionController(options);
+
+      const callActionSpy = jest
+        .spyOn(messenger, 'call')
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        .mockImplementationOnce(async (...args: any) => {
+          const [, { requestData }] = args;
+          return {
+            metadata: { ...requestData.metadata },
+            permissions: {
+              [PermissionNames.wallet_getSecretArray]: {
+                parentCapability: PermissionNames.wallet_getSecretArray,
+              },
+              [PermissionNames.wallet_getSecretObject]: {
+                parentCapability: PermissionNames.wallet_getSecretObject,
+                caveats: 'foo', // invalid
+              },
+            },
+          };
+        });
+
+      await expect(
+        async () =>
+          await controller.requestPermissions(
+            { origin },
+            {
+              [PermissionNames.wallet_getSecretArray]: {
+                parentCapability: PermissionNames.wallet_getSecretArray,
+              },
+            },
+          ),
+      ).rejects.toThrow(
+        errors.internalError(
+          `Invalid approved permissions request: The "caveats" property of permission for "${PermissionNames.wallet_getSecretObject}" of subject "${origin}" is invalid. It must be a non-empty array if specified.`,
+        ),
+      );
+
+      expect(callActionSpy).toHaveBeenCalledTimes(1);
+      expect(callActionSpy).toHaveBeenCalledWith(
+        'ApprovalController:addRequest',
+        {
+          id: expect.any(String),
+          origin,
+          requestData: {
+            metadata: { id: expect.any(String), origin },
+            permissions: {
+              [PermissionNames.wallet_getSecretArray]: {
+                parentCapability: PermissionNames.wallet_getSecretArray,
+              },
+            },
+          },
+          type: MethodNames.requestPermissions,
+        },
+        true,
+      );
+    });
   });
 
   describe('acceptPermissionsRequest', () => {

packages/permission-controller/src/PermissionController.ts

@@ -2159,14 +2159,15 @@ export class PermissionController<
     try {
       this.validateRequestedPermissions(origin, permissions);
     } catch (error) {
-      if (error instanceof JsonRpcError) {
+      if (error instanceof Error) {
         // Re-throw as an internal error; we should never receive invalid approved
         // permissions.
         throw internalError(
           `Invalid approved permissions request: ${error.message}`,
-          error.data,
+          error instanceof JsonRpcError ? error.data : undefined,
         );
       }
+      /* istanbul ignore next: We should only throw well-formed errors */
       throw internalError('Unrecognized error type', { error });
     }
   }