fix: Properly catch cronjob errors during initialization (#3373)

This PR fixes two issues:

Firstly, we were not catching errors thrown during the "daily checkin"
of the `CronjobController`, this means that if one Snap failed we would
not complete the initialization of the controller and no cronjobs would
run.

Secondly, we were running all the cronjobs sequentially during "daily
check" which seems unintentional given that we don't need the result of
the cronjob invocation.

Author: FrederikBolding

packages/snaps-controllers/coverage.json

@@ -1,6 +1,6 @@
 {
   "branches": 93.51,
-  "functions": 97.36,
-  "lines": 98.33,
-  "statements": 98.17
+  "functions": 98.14,
+  "lines": 98.48,
+  "statements": 98.31
 }

packages/snaps-controllers/src/cronjob/CronjobController.test.ts

@@ -91,7 +91,7 @@ describe('CronjobController', () => {
     cronjobController.destroy();
   });
 
-  it('executes cronjobs that were missed during daily check in', async () => {
+  it('executes cronjobs that were missed during daily check in', () => {
     const rootMessenger = getRootCronjobControllerMessenger();
     const controllerMessenger =
       getRestrictedCronjobControllerMessenger(rootMessenger);
@@ -118,7 +118,7 @@ describe('CronjobController', () => {
       };
     });
 
-    await cronjobController.dailyCheckIn();
+    cronjobController.dailyCheckIn();
 
     jest.advanceTimersByTime(inMilliseconds(24, Duration.Hour));
 
@@ -199,6 +199,38 @@ describe('CronjobController', () => {
     cronjobController2.destroy();
   });
 
+  it('catches errors during daily check in', () => {
+    const rootMessenger = getRootCronjobControllerMessenger();
+    const controllerMessenger =
+      getRestrictedCronjobControllerMessenger(rootMessenger);
+
+    rootMessenger.registerActionHandler(
+      'PermissionController:getPermissions',
+      () => {
+        return { [SnapEndowments.Cronjob]: getCronjobPermission() };
+      },
+    );
+
+    const handleRequest = jest.fn().mockRejectedValue('Snap failed to boot.');
+
+    rootMessenger.registerActionHandler(
+      'SnapController:handleRequest',
+      handleRequest,
+    );
+
+    const cronjobController = new CronjobController({
+      messenger: controllerMessenger,
+    });
+
+    cronjobController.dailyCheckIn();
+
+    jest.advanceTimersByTime(inMilliseconds(24, Duration.Hour));
+
+    expect(handleRequest).toHaveBeenCalledTimes(2);
+
+    cronjobController.destroy();
+  });
+
   it('does not schedule cronjob that is too far in the future', () => {
     const expression = '59 23 29 2 *'; // At 11:59pm on February 29th
 

packages/snaps-controllers/src/cronjob/CronjobController.ts

@@ -196,9 +196,7 @@ export class CronjobController extends BaseController<
       (...args) => this.getBackgroundEvents(...args),
     );
 
-    this.dailyCheckIn().catch((error) => {
-      logError(error);
-    });
+    this.dailyCheckIn();
 
     this.#rescheduleBackgroundEvents(Object.values(this.state.events));
   }
@@ -471,7 +469,7 @@ export class CronjobController extends BaseController<
    *
    * This is necessary for longer running jobs that execute with more than 24 hours between them.
    */
-  async dailyCheckIn() {
+  dailyCheckIn() {
     const jobs = this.#getAllJobs();
 
     for (const job of jobs) {
@@ -483,7 +481,9 @@ export class CronjobController extends BaseController<
         parsed.hasPrev() &&
         parsed.prev().getTime() > lastRun
       ) {
-        await this.#executeCronjob(job);
+        this.#executeCronjob(job).catch((error) => {
+          logError(error);
+        });
       }
 
       // Try scheduling, will fail if an existing scheduled job is found
@@ -492,10 +492,7 @@ export class CronjobController extends BaseController<
 
     this.#dailyTimer = new Timer(DAILY_TIMEOUT);
     this.#dailyTimer.start(() => {
-      this.dailyCheckIn().catch((error) => {
-        // TODO: Decide how to handle errors.
-        logError(error);
-      });
+      this.dailyCheckIn();
     });
   }