fix: Prevent double scheduling events and ensure long-running events work correctly (#3561)

Fixes two issues with the `CronjobController`. We were double scheduling
events that had not run since last boot since `#execute` also schedules
the next call. Additionally we were not correctly scheduling events that
run for longer than 24 hours, since the daily timer would re-calculate
the execution date every day.

Author: FrederikBolding

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

@@ -275,19 +275,14 @@ describe('CronjobController', () => {
       },
     );
 
-    const secondCronjobController = new CronjobController({
-      messenger: controllerMessenger,
-      state: cronjobController.state,
-      stateManager: getMockStateManager(),
-    });
+    expect(handleRequest).toHaveBeenCalledTimes(1);
 
-    secondCronjobController.init();
+    jest.advanceTimersByTime(inMilliseconds(26, Duration.Hour));
 
     await new Promise((resolve) => originalProcessNextTick(resolve));
-    expect(handleRequest).toHaveBeenCalledTimes(1);
+    expect(handleRequest).toHaveBeenCalledTimes(2);
 
     cronjobController.destroy();
-    secondCronjobController.destroy();
   });
 
   it('does not schedule cronjob that is too far in the future', () => {
@@ -473,55 +468,6 @@ describe('CronjobController', () => {
     cronjobController.destroy();
   });
 
-  it('handles scheduled event close to current time gracefully', () => {
-    const rootMessenger = getRootCronjobControllerMessenger();
-    const controllerMessenger =
-      getRestrictedCronjobControllerMessenger(rootMessenger);
-
-    const cronjobController = new CronjobController({
-      messenger: controllerMessenger,
-      stateManager: getMockStateManager(),
-      state: {
-        events: {
-          foo: {
-            id: 'foo',
-            recurring: false,
-            date: '2022-01-01T00:00:01.000Z',
-            schedule: '2022-01-01T00:00:01.000Z',
-            scheduledAt: new Date().toISOString(),
-            snapId: MOCK_SNAP_ID,
-            request: {
-              method: 'handleEvent',
-              params: ['p1'],
-            },
-          },
-        },
-      },
-    });
-
-    rootMessenger.subscribe('CronjobController:stateChange', () => {
-      jest.advanceTimersByTime(inMilliseconds(2, Duration.Second));
-    });
-
-    cronjobController.init();
-
-    expect(rootMessenger.call).toHaveBeenNthCalledWith(
-      1,
-      'SnapController:handleRequest',
-      {
-        snapId: MOCK_SNAP_ID,
-        origin: METAMASK_ORIGIN,
-        handler: HandlerType.OnCronjob,
-        request: {
-          method: 'handleEvent',
-          params: ['p1'],
-        },
-      },
-    );
-
-    cronjobController.destroy();
-  });
-
   it('handles the `snapInstalled` event', () => {
     const rootMessenger = getRootCronjobControllerMessenger();
     const controllerMessenger =
@@ -663,7 +609,7 @@ describe('CronjobController', () => {
       foo: {
         id: 'foo',
         recurring: false,
-        date: '2022-01-01T01:00:00Z',
+        date: '2022-01-01T01:00Z',
         schedule: '2022-01-01T01:00Z',
         scheduledAt: new Date().toISOString(),
         snapId: MOCK_SNAP_ID,

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

@@ -406,8 +406,15 @@ export class CronjobController extends BaseController<
    * Get the next execution date for a given event and start a timer for it.
    *
    * @param event - The event to schedule.
+   * @param next - Whether to schedule to the next date, otherwise will
+   * schedule for existing date.
    */
-  #schedule(event: InternalBackgroundEvent) {
+  #schedule(event: InternalBackgroundEvent, next = true) {
+    if (!next) {
+      this.#startTimer(event);
+      return;
+    }
+
     const date = getExecutionDate(event.schedule);
     const { nextState } = this.update((state) => {
       state.events[event.id].date = date;
@@ -621,9 +628,10 @@ export class CronjobController extends BaseController<
       // immediately.
       if (event.recurring && eventDate <= now) {
         this.#execute(event);
+        continue;
       }
 
-      this.#schedule(event);
+      this.#schedule(event, false);
     }
   }