Skip to content

Commit

Permalink
fix: HTTP recovery in the same tab (#876)
Browse files Browse the repository at this point in the history 
Co-authored-by: Andrew Bueide <andrew@abueide.com>
  • Loading branch information
Andrew Bueide committed May 13, 2020
1 parent eda02e7 commit 0ce7974
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 51 deletions.
31 changes: 6 additions & 25 deletions add-on/src/lib/ipfs-request.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ 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
Expand Down Expand Up @@ -369,8 +368,7 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
url.hostname = `${url.hostname}.link`
const redirectUrl = url.toString()
log(`onErrorOccurred: attempting to recover from DNS error (${request.error}) using EthDNS for ${request.url} → ${redirectUrl}`, request)
// TODO: update existing tab
return createTabWithURL(request, redirectUrl, browser, recoveredTabs)
return updateTabWithURL(request, redirectUrl, browser)
}

// Check if error can be recovered via DNSLink
Expand All @@ -380,7 +378,7 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
if (dnslink) {
const redirectUrl = dnslinkResolver.dnslinkAtGateway(request.url, dnslink)
log(`onErrorOccurred: attempting to recover from network error (${request.error}) using dnslink for ${request.url} → ${redirectUrl}`, request)
return createTabWithURL(request, redirectUrl, browser, recoveredTabs)
return updateTabWithURL(request, redirectUrl, browser)
}
}

Expand All @@ -394,7 +392,7 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
if (isRecoverable(request, state, ipfsPathValidator)) {
const redirectUrl = ipfsPathValidator.resolveToPublicUrl(request.url)
log(`onErrorOccurred: attempting to recover from network error (${request.error}) for ${request.url} → ${redirectUrl}`, request)
return createTabWithURL(request, redirectUrl, browser, recoveredTabs)
return updateTabWithURL(request, redirectUrl, browser)
}
},

Expand Down Expand Up @@ -424,7 +422,7 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
if (isRecoverable(request, state, ipfsPathValidator)) {
const redirectUrl = ipfsPathValidator.resolveToPublicUrl(request.url)
log(`onCompleted: attempting to recover from HTTP Error ${request.statusCode} for ${request.url} → ${redirectUrl}`, request)
return createTabWithURL(request, redirectUrl, browser, recoveredTabs)
return updateTabWithURL(request, redirectUrl, browser)
}
}
}
Expand Down Expand Up @@ -608,29 +606,12 @@ 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
// TODO: display an user-friendly prompt when the very first recovery is done
async function createTabWithURL (request, redirectUrl, browser, recoveredTabs) {
async function updateTabWithURL (request, redirectUrl, browser) {
// Do nothing if the URL remains the same
if (request.url === redirectUrl) return

const tabKey = 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({
return browser.tabs.update({
active: true,
openerTabId,
url: redirectUrl
})
if (newTab) recoveredTabs.set(tabKey, newTab.id)
}
52 changes: 26 additions & 26 deletions test/functional/lib/ipfs-request-gateway-recover.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,47 +49,47 @@ describe('requestHandler.onCompleted:', function () { // HTTP-level errors
it('should do nothing if broken request is for the default subdomain gateway', async function () {
const request = urlRequestWithStatus('https://QmYzZgeWE7r8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h.ipfs.dweb.link/wiki/', 500)
await requestHandler.onCompleted(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should redirect to default subdomain gateway on broken subdomain gateway request', async function () {
const request = urlRequestWithStatus('http://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.brokenexample.com/wiki/', 500)
await requestHandler.onCompleted(request)
assert.ok(browser.tabs.create.withArgs({ url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true, openerTabId: 20 }).calledOnce, 'tabs.create should be called with default subdomain gateway URL')
assert.ok(browser.tabs.update.withArgs({ url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL')
})
it('should do nothing if broken request is a non-IPFS request', async function () {
const request = urlRequestWithStatus('https://wikipedia.org', 500)
await requestHandler.onCompleted(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should do nothing if broken request is a local request to 127.0.0.1/ipfs', async function () {
const request = urlRequestWithStatus('http://127.0.0.1:8080/ipfs/QmYzZgeWE7r8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', 500)
await requestHandler.onCompleted(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should do nothing if broken request is a local request to localhost/ipfs', async function () {
const request = urlRequestWithStatus('http://localhost:8080/ipfs/QmYzZgeWE7r8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', 500)
await requestHandler.onCompleted(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should do nothing if broken request is a local request to *.ipfs.localhost', async function () {
const request = urlRequestWithStatus('http://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhw.ipfs.localhost:8080/', 500)
await requestHandler.onCompleted(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should do nothing if broken request is to the default public gateway', async function () {
const request = urlRequestWithStatus('https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', 500)
await requestHandler.onCompleted(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should do nothing if broken request is not a \'main_frame\' request', async function () {
const request = urlRequestWithStatus('https://nondefaultipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', 500, 'stylesheet')
await requestHandler.onCompleted(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should recover from unreachable third party public gateway by reopening on the public gateway', async function () {
const request = urlRequestWithStatus('https://nondefaultipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', 500)
await requestHandler.onCompleted(request)
assert.ok(browser.tabs.create.withArgs({ url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true, openerTabId: 20 }).calledOnce, 'tabs.create should be called with IPFS default public gateway URL')
assert.ok(browser.tabs.update.withArgs({ url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL')
})
})

Expand All @@ -101,17 +101,17 @@ describe('requestHandler.onCompleted:', function () { // HTTP-level errors
it('should do nothing on failed subdomain gateway request', async function () {
const request = urlRequestWithStatus('https://QmYzZgeWE7r8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h.ipfs.brokendomain.com/wiki/', 500)
await requestHandler.onCompleted(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should do nothing on broken non-default public gateway IPFS request', async function () {
const request = urlRequestWithStatus('https://nondefaultipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', 500)
await requestHandler.onCompleted(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
})

afterEach(function () {
browser.tabs.create.reset()
browser.tabs.update.reset()
})

after(function () {
Expand Down Expand Up @@ -148,38 +148,38 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors
it('should do nothing if failed request is for the default subdomain gateway', async function () {
const request = urlRequestWithStatus('https://QmYzZgeWE7r8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h.ipfs.dweb.link/wiki/', 500)
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should redirect to default subdomain gateway on failed subdomain gateway request', async function () {
const request = urlRequestWithStatus('http://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.brokenexample.com/wiki/', 500)
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.create.withArgs({ url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true, openerTabId: 20 }).calledOnce, 'tabs.create should be called with default subdomain gateway URL')
assert.ok(browser.tabs.update.withArgs({ url: 'https://bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq.ipfs.dweb.link/wiki/', active: true }).calledOnce, 'tabs.update should be called with default subdomain gateway URL')
})
it('should do nothing if failed request is a non-IPFS request', async function () {
const request = urlRequestWithNetworkError('https://wikipedia.org')
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should do nothing if failed request is a non-public IPFS request', async function () {
const request = urlRequestWithNetworkError('http://127.0.0.1:8080/ipfs/QmYzZgeWE7r8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h')
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should do nothing if failed request is to the default public gateway', async function () {
const request = urlRequestWithNetworkError('https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h')
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should do nothing if failed request is not a \'main_frame\' request', async function () {
const requestType = 'stylesheet'
const request = urlRequestWithNetworkError('https://nondefaultipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', 'net::ERR_NAME_NOT_RESOLVED', requestType)
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should recover from unreachable third party public gateway by reopening on the public gateway', async function () {
const request = urlRequestWithNetworkError('https://nondefaultipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h')
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.create.withArgs({ url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true, openerTabId: 20 }).calledOnce, 'tabs.create should be called with IPFS default public gateway URL')
assert.ok(browser.tabs.update.withArgs({ url: 'https://ipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h', active: true }).calledOnce, 'tabs.update should be called with IPFS default public gateway URL')
})
it('should recover from unreachable HTTP server by reopening DNSLink on the active gateway', async function () {
state.dnslinkPolicy = 'best-effort'
Expand All @@ -188,7 +188,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors
const expectedUrl = 'http://localhost:8080/ipns/en.wikipedia-on-ipfs.org/'
const request = urlRequestWithNetworkError('https://en.wikipedia-on-ipfs.org/')
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.create.withArgs({ url: expectedUrl, active: true, openerTabId: 20 }).calledOnce, 'tabs.create should be called with DNSLink on local gateway URL')
assert.ok(browser.tabs.update.withArgs({ url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with DNSLink on local gateway URL')
dnslinkResolver.clearCache()
})
it('should recover from failed DNS for .eth opening it on EthDNS gateway at .eth.link', async function () {
Expand All @@ -199,7 +199,7 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors
const expectedUrl = 'https://almonit.eth.link/'
const request = urlRequestWithNetworkError('https://almonit.eth', dnsFailure)
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.create.withArgs({ url: expectedUrl, active: true, openerTabId: 20 }).calledOnce, 'tabs.create should be called with ENS resource on local gateway URL')
assert.ok(browser.tabs.update.withArgs({ url: expectedUrl, active: true }).calledOnce, 'tabs.update should be called with ENS resource on local gateway URL')
dnslinkResolver.clearCache()
})
})
Expand All @@ -212,19 +212,19 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors
it('should do nothing on unreachable third party public gateway', async function () {
const request = urlRequestWithNetworkError('https://nondefaultipfs.io/ipfs/QmYbZgeWE7y8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h')
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should do nothing on failed subdomain gateway request', async function () {
const request = urlRequestWithStatus('https://QmYzZgeWE7r8HXkH8zbb8J9ddHQvp8LTqm6isL791eo14h.ipfs.brokendomain.com/wiki/', 500)
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
})
it('should do nothing on unreachable HTTP server with DNSLink', async function () {
state.dnslinkPolicy = 'best-effort'
dnslinkResolver.setDnslink('en.wikipedia-on-ipfs.org', '/ipfs/QmXoypizjW3WknFiJnKLwHCnL72vedxjQkDDP1mXWo6uco')
const request = urlRequestWithNetworkError('https://en.wikipedia-on-ipfs.org')
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
dnslinkResolver.clearCache()
})
it('should do nothing on failed non-default public gateway IPFS request', async function () {
Expand All @@ -234,13 +234,13 @@ describe('requestHandler.onErrorOccurred:', function () { // network errors
const dnsFailure = 'net::ERR_NAME_NOT_RESOLVED' // chrome code
const request = urlRequestWithNetworkError('https://almonit.eth', dnsFailure)
await requestHandler.onErrorOccurred(request)
assert.ok(browser.tabs.create.notCalled, 'tabs.create should not be called')
assert.ok(browser.tabs.update.notCalled, 'tabs.update should not be called')
dnslinkResolver.clearCache()
})
})

afterEach(function () {
browser.tabs.create.reset()
browser.tabs.update.reset()
})

after(function () {
Expand Down

0 comments on commit 0ce7974

Please sign in to comment.