From 33a16b85e412abbf54f22d21feb97e876e5dd784 Mon Sep 17 00:00:00 2001 From: Shin'ya Ueoka Date: Tue, 30 Jul 2019 21:52:22 +0900 Subject: [PATCH 1/2] Handle errors on loading settings The error on loading settings can occurs when the settings lose backward compatibility on version up, or the saved date is broken. The error is caught, then the script done fallback to default settings and notify it to user. --- .eslintrc | 1 + src/background/presenters/NotifyPresenter.ts | 15 ++++++++++++++- src/background/usecases/SettingUseCase.ts | 12 ++++++++++-- src/background/usecases/VersionUseCase.ts | 4 +--- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/.eslintrc b/.eslintrc index 7b35bc4..9667b89 100644 --- a/.eslintrc +++ b/.eslintrc @@ -33,6 +33,7 @@ "function-paren-newline": "off", "id-length": "off", "indent": ["error", 2], + "init-declarations": "off", "jsx-quotes": ["error", "prefer-single"], "max-classes-per-file": "off", "max-lines": "off", diff --git a/src/background/presenters/NotifyPresenter.ts b/src/background/presenters/NotifyPresenter.ts index 8fa4acb..6498fbf 100644 --- a/src/background/presenters/NotifyPresenter.ts +++ b/src/background/presenters/NotifyPresenter.ts @@ -4,7 +4,20 @@ const NOTIFICATION_ID = 'vimvixen-update'; @injectable() export default class NotifyPresenter { - async notify( + async notifyUpdated(version: string, onclick: () => void): Promise { + let title = `Vim Vixen ${version} has been installed`; + let message = 'Click here to see release notes'; + await this.notify(title, message, onclick); + } + + async notifyInvalidSettings(onclick: () => void): Promise { + let title = `Loaded settings is invalid`; + // eslint-disable-next-line max-len + let message = 'The default settings is used due to the last saved settings is invalid. Check your current settings from the add-on preference'; + await this.notify(title, message, onclick); + } + + private async notify( title: string, message: string, onclick: () => void, diff --git a/src/background/usecases/SettingUseCase.ts b/src/background/usecases/SettingUseCase.ts index fd00f80..694d3c1 100644 --- a/src/background/usecases/SettingUseCase.ts +++ b/src/background/usecases/SettingUseCase.ts @@ -4,6 +4,7 @@ import PersistentSettingRepository import SettingRepository from '../repositories/SettingRepository'; import { DefaultSettingData } from '../../shared/SettingData'; import Settings from '../../shared/Settings'; +import NotifyPresenter from '../presenters/NotifyPresenter'; @injectable() export default class SettingUseCase { @@ -11,6 +12,7 @@ export default class SettingUseCase { constructor( private persistentSettingRepository: PersistentSettingRepository, private settingRepository: SettingRepository, + private notifyPresenter: NotifyPresenter, ) { } @@ -24,8 +26,14 @@ export default class SettingUseCase { data = DefaultSettingData; } - let value = data.toSettings(); - this.settingRepository.update(value); + let value: Settings; + try { + value = data.toSettings(); + } catch (e) { + this.notifyPresenter.notifyInvalidSettings(() => {}); + value = DefaultSettingData.toSettings(); + } + this.settingRepository.update(value!!); return value; } } diff --git a/src/background/usecases/VersionUseCase.ts b/src/background/usecases/VersionUseCase.ts index 1d9ef8b..0ff0e9b 100644 --- a/src/background/usecases/VersionUseCase.ts +++ b/src/background/usecases/VersionUseCase.ts @@ -12,10 +12,8 @@ export default class VersionUseCase { notify(): Promise { let manifest = browser.runtime.getManifest(); - let title = `Vim Vixen ${manifest.version} has been installed`; - let message = 'Click here to see release notes'; let url = this.releaseNoteUrl(manifest.version); - return this.notifyPresenter.notify(title, message, () => { + return this.notifyPresenter.notifyUpdated(manifest.version, () => { this.tabPresenter.create(url); }); } From 9b2b8f0608df9e4c7a251f49d8ed818b8966786e Mon Sep 17 00:00:00 2001 From: Shin'ya Ueoka Date: Thu, 1 Aug 2019 21:56:28 +0900 Subject: [PATCH 2/2] Distinct notification IDs --- src/background/presenters/NotifyPresenter.ts | 36 +++++++++----------- src/background/usecases/SettingUseCase.ts | 2 +- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/background/presenters/NotifyPresenter.ts b/src/background/presenters/NotifyPresenter.ts index 6498fbf..9785278 100644 --- a/src/background/presenters/NotifyPresenter.ts +++ b/src/background/presenters/NotifyPresenter.ts @@ -1,39 +1,37 @@ import { injectable } from 'tsyringe'; -const NOTIFICATION_ID = 'vimvixen-update'; +const NOTIFICATION_ID_UPDATE = 'vimvixen-update'; +const NOTIFICATION_ID_INVALID_SETTINGS = 'vimvixen-update-invalid-settings'; @injectable() export default class NotifyPresenter { async notifyUpdated(version: string, onclick: () => void): Promise { let title = `Vim Vixen ${version} has been installed`; let message = 'Click here to see release notes'; - await this.notify(title, message, onclick); - } - - async notifyInvalidSettings(onclick: () => void): Promise { - let title = `Loaded settings is invalid`; - // eslint-disable-next-line max-len - let message = 'The default settings is used due to the last saved settings is invalid. Check your current settings from the add-on preference'; - await this.notify(title, message, onclick); - } - private async notify( - title: string, - message: string, - onclick: () => void, - ): Promise { const listener = (id: string) => { - if (id !== NOTIFICATION_ID) { + if (id !== NOTIFICATION_ID_UPDATE) { return; } - onclick(); - browser.notifications.onClicked.removeListener(listener); }; browser.notifications.onClicked.addListener(listener); - await browser.notifications.create(NOTIFICATION_ID, { + await browser.notifications.create(NOTIFICATION_ID_UPDATE, { + 'type': 'basic', + 'iconUrl': browser.extension.getURL('resources/icon_48x48.png'), + title, + message, + }); + } + + async notifyInvalidSettings(): Promise { + let title = `Loaded settings is invalid`; + // eslint-disable-next-line max-len + let message = 'The default settings is used due to the last saved settings is invalid. Check your current settings from the add-on preference'; + + await browser.notifications.create(NOTIFICATION_ID_INVALID_SETTINGS, { 'type': 'basic', 'iconUrl': browser.extension.getURL('resources/icon_48x48.png'), title, diff --git a/src/background/usecases/SettingUseCase.ts b/src/background/usecases/SettingUseCase.ts index 694d3c1..53c8f1d 100644 --- a/src/background/usecases/SettingUseCase.ts +++ b/src/background/usecases/SettingUseCase.ts @@ -30,7 +30,7 @@ export default class SettingUseCase { try { value = data.toSettings(); } catch (e) { - this.notifyPresenter.notifyInvalidSettings(() => {}); + this.notifyPresenter.notifyInvalidSettings(); value = DefaultSettingData.toSettings(); } this.settingRepository.update(value!!);