From 04c077d614b3eae95da1e9b506171a87948a4179 Mon Sep 17 00:00:00 2001
From: Shin'ya Ueoka <ueokande@i-beam.org>
Date: Tue, 27 Aug 2019 22:33:33 +0900
Subject: [PATCH] Fix undefined checking on operation parameter

---
 src/background/usecases/SettingUseCase.ts | 18 ++++++++++++----
 src/shared/operations.ts                  | 26 +++++++++++++----------
 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/src/background/usecases/SettingUseCase.ts b/src/background/usecases/SettingUseCase.ts
index ee131c7..d73521f 100644
--- a/src/background/usecases/SettingUseCase.ts
+++ b/src/background/usecases/SettingUseCase.ts
@@ -21,7 +21,12 @@ export default class SettingUseCase {
   }
 
   async reload(): Promise<Settings> {
-    let data = await this.persistentSettingRepository.load();
+    let data;
+    try {
+      data = await this.persistentSettingRepository.load();
+    } catch (e) {
+      this.showUnableToLoad(e);
+    }
     if (!data) {
       data = DefaultSettingData;
     }
@@ -30,12 +35,17 @@ export default class SettingUseCase {
     try {
       value = data.toSettings();
     } catch (e) {
-      this.notifyPresenter.notifyInvalidSettings(() => {
-        browser.runtime.openOptionsPage();
-      });
+      this.showUnableToLoad(e);
       value = DefaultSettingData.toSettings();
     }
     this.settingRepository.update(value!!);
     return value;
   }
+
+  private showUnableToLoad(e: Error) {
+    console.error('unable to load settings', e);
+    this.notifyPresenter.notifyInvalidSettings(() => {
+      browser.runtime.openOptionsPage();
+    });
+  }
 }
diff --git a/src/shared/operations.ts b/src/shared/operations.ts
index 2df2e67..1ce5256 100644
--- a/src/shared/operations.ts
+++ b/src/shared/operations.ts
@@ -368,7 +368,9 @@ export type Operation =
 const assertOptionalBoolean = (obj: any, name: string) => {
   if (Object.prototype.hasOwnProperty.call(obj, name) &&
       typeof obj[name] !== 'boolean') {
-    throw new TypeError(`Not a boolean parameter: '${name}'`);
+    throw new TypeError(
+      `Not a boolean parameter: '${name} (${typeof obj[name]})'`,
+    );
   }
 };
 
@@ -376,7 +378,9 @@ const assertOptionalString = (obj: any, name: string, values?: string[]) => {
   if (Object.prototype.hasOwnProperty.call(obj, name)) {
     let value = obj[name];
     if (typeof value !== 'string') {
-      throw new TypeError(`Not a string parameter: '${name}'`);
+      throw new TypeError(
+        `Not a string parameter: '${name}' (${typeof value})`,
+      );
     }
     if (values && values.length && values.indexOf(value) === -1) {
       // eslint-disable-next-line max-len
@@ -421,32 +425,32 @@ export const valueOf = (o: any): Operation => {
     assertOptionalBoolean(o, 'background');
     return {
       type: FOLLOW_START,
-      newTab: Boolean(typeof o.newTab === undefined ? false : o.newTab),
-      background: Boolean(typeof o.background === undefined ? true : o.background), // eslint-disable-line max-len
+      newTab: Boolean(typeof o.newTab === 'undefined' ? false : o.newTab),
+      background: Boolean(typeof o.background === 'undefined' ? true : o.background), // eslint-disable-line max-len
     };
   case PAGE_HOME:
     assertOptionalBoolean(o, 'newTab');
     return {
       type: PAGE_HOME,
-      newTab: Boolean(typeof o.newTab === undefined ? false : o.newTab),
+      newTab: Boolean(typeof o.newTab === 'undefined' ? false : o.newTab),
     };
   case TAB_CLOSE:
     assertOptionalString(o, 'select', ['left', 'right']);
     return {
       type: TAB_CLOSE,
-      select: (typeof o.select === undefined ? 'right' : o.select),
+      select: (typeof o.select === 'undefined' ? 'right' : o.select),
     };
   case TAB_RELOAD:
     assertOptionalBoolean(o, 'cache');
     return {
       type: TAB_RELOAD,
-      cache: Boolean(typeof o.cache === undefined ? false : o.cache),
+      cache: Boolean(typeof o.cache === 'undefined' ? false : o.cache),
     };
   case URLS_PASTE:
     assertOptionalBoolean(o, 'newTab');
     return {
       type: URLS_PASTE,
-      newTab: Boolean(typeof o.newTab === undefined ? false : o.newTab),
+      newTab: Boolean(typeof o.newTab === 'undefined' ? false : o.newTab),
     };
   case INTERNAL_OPEN_URL:
     assertOptionalBoolean(o, 'newTab');
@@ -456,9 +460,9 @@ export const valueOf = (o: any): Operation => {
     return {
       type: INTERNAL_OPEN_URL,
       url: o.url,
-      newTab: Boolean(typeof o.newTab === undefined ? false : o.newTab),
-      newWindow: Boolean(typeof o.newWindow === undefined ? false : o.newWindow), // eslint-disable-line max-len
-      background: Boolean(typeof o.background === undefined ? true : o.background), // eslint-disable-line max-len
+      newTab: Boolean(typeof o.newTab === 'undefined' ? false : o.newTab),
+      newWindow: Boolean(typeof o.newWindow === 'undefined' ? false : o.newWindow), // eslint-disable-line max-len
+      background: Boolean(typeof o.background === 'undefined' ? true : o.background), // eslint-disable-line max-len
     };
   case CANCEL:
   case ADDON_ENABLE: