fix: recovery quirks in Firefox and Chromium (#807)

This fixes small quirks that did not break recovery,
but made it bit confusing or inefficient. 

* fix: duplicated recovery tabs in Chromium

Deduplicate `onErrorOccurred` events: sometimes Chrome produces two, possibly due to lack of async functions in webRequest APIs

Closes #805

* fix: skip requests from DNS fixup logic in Firefox

Firefox automatically adds 'www.' to TLD+1 names if initial DNS lookup failed.
This produced bogus request+error which recovery logic is now aware of and ignores.

Closes #804

* Other

- make `onErrorOccurred` non-async (only Firefox supports those)
- listen only for errors when `webRequest.ResourceType`=`main_frame`
- reuse existing tab

Author: lidel

add-on/src/lib/ipfs-companion.js

@@ -105,8 +105,8 @@ module.exports = async function init () {
     browser.webRequest.onBeforeSendHeaders.addListener(onBeforeSendHeaders, { urls: ['<all_urls>'] }, onBeforeSendInfoSpec)
     browser.webRequest.onBeforeRequest.addListener(onBeforeRequest, { urls: ['<all_urls>'] }, ['blocking'])
     browser.webRequest.onHeadersReceived.addListener(onHeadersReceived, { urls: ['<all_urls>'] }, ['blocking', 'responseHeaders'])
-    browser.webRequest.onErrorOccurred.addListener(onErrorOccurred, { urls: ['<all_urls>'] })
-    browser.webRequest.onCompleted.addListener(onCompleted, { urls: ['<all_urls>'] })
+    browser.webRequest.onErrorOccurred.addListener(onErrorOccurred, { urls: ['<all_urls>'], types: ['main_frame'] })
+    browser.webRequest.onCompleted.addListener(onCompleted, { urls: ['<all_urls>'], types: ['main_frame'] })
     browser.storage.onChanged.addListener(onStorageChange)
     browser.webNavigation.onCommitted.addListener(onNavigationCommitted)
     browser.webNavigation.onDOMContentLoaded.addListener(onDOMContentLoaded)

add-on/src/lib/ipfs-request.js

@@ -33,9 +33,11 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
 
   // Various types of requests are identified once and cached across all browser.webRequest hooks
   const requestCacheCfg = { max: 128, maxAge: 1000 * 30 }
+  const recoveredTabs = new LRU(requestCacheCfg)
   const ignoredRequests = new LRU(requestCacheCfg)
   const ignore = (id) => ignoredRequests.set(id, true)
   const isIgnored = (id) => ignoredRequests.get(id) !== undefined
+  const errorInFlight = new LRU({ max: 3, maxAge: 1000 })
 
   const acrhHeaders = new LRU(requestCacheCfg) // webui cors fix in Chrome
   const originUrls = new LRU(requestCacheCfg) // request.originUrl workaround for Chrome
@@ -365,17 +367,30 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
     // browser.webRequest.onErrorOccurred
     // Fired when a request could not be processed due to an error on network level.
     // For example: TCP timeout, DNS lookup failure
-    async onErrorOccurred (request) {
+    // NOTE: this is executed only if webRequest.ResourceType='main_frame'
+    onErrorOccurred (request) {
       const state = getState()
       if (!state.active) return
 
+      // Avoid duplicates in Chromium, which fires two events instead of one
+      // https://github.com/ipfs-shipyard/ipfs-companion/issues/805
+      if (errorInFlight.has(request.url)) return
+      errorInFlight.set(request.url, request.requestId)
+
+      // Skip additional requests produced by DNS fixup logic in Firefox
+      // https://github.com/ipfs-shipyard/ipfs-companion/issues/804
+      if (request.error === 'NS_ERROR_UNKNOWN_HOST' && request.url.includes('://www.')) {
+        const urlBeforeFixup = request.url.replace('://www.', '://')
+        if (errorInFlight.has(urlBeforeFixup)) return
+      }
+
       // Check if error can be recovered via EthDNS
       if (isRecoverableViaEthDNS(request, state)) {
         const url = new URL(request.url)
         url.hostname = `${url.hostname}.link`
         const redirect = { redirectUrl: url.toString() }
-        log(`onErrorOccurred: attempting to recover from DNS error (${request.error}) using EthDNS for ${request.url}`, redirect.redirectUrl)
-        return createTabWithURL(redirect, browser)
+        log(`onErrorOccurred: attempting to recover from DNS error (${request.error}) using EthDNS for ${request.url} → ${redirect.redirectUrl}`, request)
+        return createTabWithURL(redirect, browser, recoveredTabs)
       }
 
       // Check if error can be recovered via DNSLink
@@ -384,8 +399,8 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
         const dnslink = dnslinkResolver.readAndCacheDnslink(hostname)
         if (dnslink) {
           const redirect = dnslinkResolver.dnslinkRedirect(request.url, dnslink)
-          log(`onErrorOccurred: attempting to recover from network error (${request.error}) using dnslink for ${request.url}`, redirect.redirectUrl)
-          return createTabWithURL(redirect, browser)
+          log(`onErrorOccurred: attempting to recover from network error (${request.error}) using dnslink for ${request.url} → ${redirect.redirectUrl}`, request)
+          return createTabWithURL(redirect, browser, recoveredTabs)
         }
       }
 
@@ -399,14 +414,15 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
         } else {
           redirectUrl = ipfsPathValidator.resolveToPublicUrl(request.url, state.pubGwURLString)
         }
-        log(`onErrorOccurred: attempting to recover from network error (${request.error}) for ${request.url}`, redirectUrl)
-        return createTabWithURL({ redirectUrl }, browser)
+        log(`onErrorOccurred: attempting to recover from network error (${request.error}) for ${request.url} → ${redirectUrl}`, request)
+        return createTabWithURL({ redirectUrl }, browser, recoveredTabs)
       }
     },
 
     // browser.webRequest.onCompleted
     // Fired when HTTP request is completed (successfully or with an error code)
-    async onCompleted (request) {
+    // NOTE: this is executed only if webRequest.ResourceType='main_frame'
+    onCompleted (request) {
       const state = getState()
       if (!state.active) return
       if (request.statusCode === 200) return // finish if no error to recover from
@@ -418,8 +434,8 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
         } else {
           redirectUrl = ipfsPathValidator.resolveToPublicUrl(request.url, state.pubGwURLString)
         }
-        log(`onCompleted: attempting to recover from HTTP Error ${request.statusCode} for ${request.url}`, redirectUrl)
-        return createTabWithURL({ redirectUrl }, browser)
+        log(`onCompleted: attempting to recover from HTTP Error ${request.statusCode} for ${request.url} → ${redirectUrl}`, request)
+        return createTabWithURL({ redirectUrl }, browser, recoveredTabs)
       }
     }
   }
@@ -560,11 +576,27 @@ function isRecoverableViaEthDNS (request, state) {
 
 // We can't redirect in onErrorOccurred/onCompleted
 // Indead, we recover by opening URL in a new tab that replaces the failed one
-async function createTabWithURL (redirect, browser) {
-  const currentTabId = await browser.tabs.query({ active: true, currentWindow: true }).then(tabs => tabs[0].id)
-  return browser.tabs.create({
+// TODO: display an user-friendly prompt when the very first recovery is done
+async function createTabWithURL (redirect, browser, recoveredTabs) {
+  const tabKey = redirect.redirectUrl
+  // reuse existing tab, if exists
+  // (this avoids duplicated tabs - https://github.com/ipfs-shipyard/ipfs-companion/issues/805)
+  try {
+    const recoveredId = recoveredTabs.get(tabKey)
+    const existingTab = recoveredId ? await browser.tabs.get(recoveredId) : undefined
+    if (existingTab) {
+      await browser.tabs.update(recoveredId, { active: true })
+      return
+    }
+  } catch (_) {
+    // tab no longer exist, let's create a new one
+  }
+  const failedTab = await browser.tabs.getCurrent()
+  const openerTabId = failedTab ? failedTab.id : undefined
+  const newTab = await browser.tabs.create({
     active: true,
-    openerTabId: currentTabId,
+    openerTabId,
     url: redirect.redirectUrl
   })
+  if (newTab) recoveredTabs.set(tabKey, newTab.id)
 }

test/functional/lib/ipfs-request-gateway-recover.test.js

@@ -27,7 +27,7 @@ describe('requestHandler.onCompleted:', function () { // HTTP-level errors
 
   before(function () {
     global.URL = URL
-    browser.tabs = { ...browser.tabs, query: sinon.stub().resolves([{ id: 20 }]) }
+    browser.tabs = { ...browser.tabs, getCurrent: sinon.stub().resolves({ id: 20 }) }
     global.browser = browser
   })