update error reporting in popup

4 files changed, 100 insertions, 33 deletions

diff --git a/common/policy.js b/common/policy.js
index 7ab9b5d..546aeed 100644
--- a/common/policy.js
+++ b/common/policy.js
@@ -66,7 +66,7 @@ function decide_policy(patterns_tree, url, default_allow, secret)
     } catch (e) {
 	console.error(e);
 	policy.allow = false;
-	policy.error = true;
+	policy.error = {haketilo_error_type: "deciding_policy"};
     }
 
     if (payloads !== undefined) {
diff --git a/html/popup.js b/html/popup.js
index b4602f3..532feba 100644
--- a/html/popup.js
+++ b/html/popup.js
@@ -79,18 +79,40 @@ function show_page_info(page_info) {
 
 	by_id("scripts_blocked").innerText = page_info.allow ? "no" : "yes";
 
-	by_id("injected_payload").innerText = page_info.payload ?
-	    page_info.payload.identifier : "None";
+	let payload_text = "None";
+
+	if (page_info.payload) {
+	    if ("error" in page_info) {
+		if (page_info.error.haketilo_error_type === "missing")
+		    payload_text = `None (error: resource with id '${page_info.error.id}' missing from the database)`;
+		else if (page_info.error.haketilo_error_type === "circular")
+		    payload_text = `None (error: circular dependency of resource with id '${page_info.error.id}' on itself)`;
+		else if (page_info.error.haketilo_error_type === "db")
+		    payload_text = `None (error: failure reading Haketilo internal database)`;
+		else if (page_info.error.haketilo_error_type === "other")
+		    payload_text = `None (error: unknown failure occured)`;
+	    } else {
+		payload_text = page_info.payload.identifier;
+	    }
+	}
+
+	by_id("injected_payload").innerText = payload_text;
 
 	const scripts_fate = page_info.allow ? "allowed" : "blocked";
 
+	let mapping_text;
+
 	if (page_info.mapping === "~allow")
-	    var mapping = `None (scripts ${scripts_fate} by a rule)`;
+	    mapping_text = `None (scripts ${scripts_fate} by a rule)`;
+	else if ("error" in page_info
+		 && page_info.error.haketilo_error_type ==="deciding_policy")
+	    mapping_text = `None (error occured when determining policy)`;
 	else if (page_info.mapping)
-	    var mapping = page_info.mapping;
-	else if (page_info.error)
-	    var mapping = `None (error occured when determining policy)`;
-	by_id("mapping_used").innerText = mapping;
+	    mapping_text = page_info.mapping;
+	else
+	    mapping_text = `None (scripts ${scripts_fate} by default policy)`;
+
+	by_id("mapping_used").innerText = mapping_text;
     }
 }
 
diff --git a/test/unit/test_policy_deciding.py b/test/unit/test_policy_deciding.py
index 88095af..75b35ac 100644
--- a/test/unit/test_policy_deciding.py
+++ b/test/unit/test_policy_deciding.py
@@ -55,7 +55,7 @@ def test_decide_policy(execute_in_page):
         returnval(decide_policy(pqt.make(), "http://unkno.wn/", true, "abcd"));
         ''')
     assert policy['allow'] == True
-    for prop in ('mapping', 'payload', 'nonce', 'csp'):
+    for prop in ('mapping', 'payload', 'nonce', 'csp', 'error'):
         assert prop not in policy
 
     policy = execute_in_page(
@@ -66,7 +66,7 @@ def test_decide_policy(execute_in_page):
         }''')
     assert policy['allow'] == True
     assert policy['mapping'] == '~allow'
-    for prop in ('payload', 'nonce', 'csp'):
+    for prop in ('payload', 'nonce', 'csp', 'error'):
         assert prop not in policy
 
     policy = execute_in_page(
@@ -75,7 +75,7 @@ def test_decide_policy(execute_in_page):
         '''
         )
     assert policy['allow'] == False
-    for prop in ('mapping', 'payload', 'nonce'):
+    for prop in ('mapping', 'payload', 'nonce', 'error'):
         assert prop not in policy
     assert parse_csp(policy['csp']) == {
         'prefetch-src':    "'none'",
@@ -92,7 +92,7 @@ def test_decide_policy(execute_in_page):
         }''')
     assert policy['allow'] == False
     assert policy['mapping'] == '~allow'
-    for prop in ('payload', 'nonce'):
+    for prop in ('payload', 'nonce', 'error'):
         assert prop not in policy
     assert parse_csp(policy['csp']) == {
         'prefetch-src':    "'none'",
@@ -110,7 +110,7 @@ def test_decide_policy(execute_in_page):
     assert policy['allow'] == False
     assert policy['mapping'] == 'm1'
     assert policy['payload'] == {'identifier': 'res1'}
-
+    assert 'error' not in policy
     assert policy['nonce'] == \
         sha256('m1:res1:http://kno.wn/:abcd'.encode()).digest().hex()
     assert parse_csp(policy['csp']) == {
@@ -119,3 +119,17 @@ def test_decide_policy(execute_in_page):
         'script-src':      f"'nonce-{policy['nonce']}'",
         'script-src-elem': f"'nonce-{policy['nonce']}'"
     }
+
+    policy = execute_in_page(
+        'returnval(decide_policy(pqt.make(), "<bad_url>", true, "abcd"));'
+    )
+    assert policy['allow'] == False
+    assert policy['error'] == {'haketilo_error_type': 'deciding_policy'}
+    for prop in ('mapping', 'payload', 'nonce'):
+        assert prop not in policy
+    assert parse_csp(policy['csp']) == {
+        'prefetch-src':    "'none'",
+        'script-src-attr': "'none'",
+        'script-src':      "'none'",
+        'script-src-elem': "'none'"
+    }
diff --git a/test/unit/test_popup.py b/test/unit/test_popup.py
index bc53e6c..da1812d 100644
--- a/test/unit/test_popup.py
+++ b/test/unit/test_popup.py
@@ -39,6 +39,12 @@ unprivileged_page_info = {
     'allow': False
 }
 
+mapping_page_info = {
+    **unprivileged_page_info,
+    'mapping': 'm1',
+    'payload': {'identifier': 'res1'}
+}
+
 mocked_page_infos = {
     'privileged': {
         'url': 'moz-extension://<some-id>/file.html',
@@ -58,14 +64,26 @@ mocked_page_infos = {
         'allow': True,
         'mapping': '~allow'
     },
-    'mapping': {
-        **unprivileged_page_info,
-        'mapping': 'm1',
-        'payload': {'identifier': 'res1'}
+    'mapping': mapping_page_info,
+    'error_deciding_policy': {
+        **mapping_page_info,
+        'error': {'haketilo_error_type': 'deciding_policy'}
     },
-    'error': {
-        **unprivileged_page_info,
-        'error': True
+    'error_missing': {
+        **mapping_page_info,
+        'error': {'haketilo_error_type': 'missing', 'id': 'some-missing-res'}
+    },
+    'error_circular': {
+        **mapping_page_info,
+        'error': {'haketilo_error_type': 'circular', 'id': 'some-circular-res'}
+    },
+    'error_db': {
+        **mapping_page_info,
+        'error': {'haketilo_error_type': 'db'}
+    },
+    'error_other': {
+        **mapping_page_info,
+        'error': {'haketilo_error_type': 'other'}
     }
 }
 
@@ -147,32 +165,45 @@ def test_popup_display(driver, execute_in_page, page_info_key):
     assert by_id['page_url'].text == mocked_page_infos[page_info_key]['url']
     assert not by_id['repo_query_container'].is_displayed()
 
-    if 'blocked' in page_info_key or page_info_key in ('mapping', 'error'):
-        assert by_id['scripts_blocked'].text.lower() == 'yes'
-    elif 'allowed' in page_info_key:
+    if 'allow' in page_info_key:
         assert by_id['scripts_blocked'].text.lower() == 'no'
+    elif page_info_key != 'privileged':
+        assert by_id['scripts_blocked'].text.lower() == 'yes'
 
+    payload_text = by_id['injected_payload'].text
     if page_info_key == 'mapping':
-        assert by_id['injected_payload'].text == 'res1'
+        assert payload_text == 'res1'
+    elif page_info_key == 'error_missing':
+        assert payload_text == \
+            "None (error: resource with id 'some-missing-res' missing from the database)"
+    elif page_info_key == 'error_circular':
+        assert payload_text == \
+            "None (error: circular dependency of resource with id 'some-circular-res' on itself)"
+    elif page_info_key == 'error_db':
+        assert payload_text == \
+            'None (error: failure reading Haketilo internal database)'
+    elif page_info_key == 'error_other':
+        assert payload_text == \
+            'None (error: unknown failure occured)'
     elif page_info_key != 'privileged':
-        assert by_id['injected_payload'].text == 'None'
+        assert payload_text == 'None'
 
     mapping_text = by_id['mapping_used'].text
-    if page_info_key == 'mapping':
+
+    if page_info_key == 'error_deciding_policy':
+        assert mapping_text == 'None (error occured when determining policy)'
+    elif page_info_key == 'mapping' or page_info_key.startswith('error'):
         assert mapping_text == 'm1'
 
     if 'allowed' in page_info_key:
-        'None (scripts allowed by' in mapping_text
+        assert 'None (scripts allowed by' in mapping_text
     elif 'blocked' in page_info_key:
-        'None (scripts blocked by' in mapping_text
+        assert 'None (scripts blocked by' in mapping_text
 
     if 'rule' in page_info_key:
-        'by a rule)' in mapping_text
+        assert 'by a rule)' in mapping_text
     elif 'default' in page_info_key:
-        'by default_policy)' in mapping_text
-
-    if page_info_key == 'error':
-        assert mapping_text == 'None (error occured when determining policy)'
+        assert 'by default policy)' in mapping_text
 
 @pytest.mark.ext_data(popup_ext_data)
 @pytest.mark.usefixtures('webextension')