diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json index f7f509342b..236fcc9d15 100644 --- a/packages/snaps-controllers/coverage.json +++ b/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 } diff --git a/packages/snaps-controllers/src/cronjob/CronjobController.test.ts b/packages/snaps-controllers/src/cronjob/CronjobController.test.ts index 750be5b616..ce9283288e 100644 --- a/packages/snaps-controllers/src/cronjob/CronjobController.test.ts +++ b/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 diff --git a/packages/snaps-controllers/src/cronjob/CronjobController.ts b/packages/snaps-controllers/src/cronjob/CronjobController.ts index 674635c223..059ee93262 100644 --- a/packages/snaps-controllers/src/cronjob/CronjobController.ts +++ b/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(); }); }