Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: reload failed IPFS tabs when API becomes available #1092

Merged
merged 19 commits into from
Sep 26, 2022
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
root=true

[*]
end_of_line = lf
insert_final_newline = true
charset = utf-8
indent_style = space
indent_size = 2
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to remove ambiguity from IDEs

63 changes: 39 additions & 24 deletions add-on/src/lib/ipfs-client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const external = require('./external')
const embedded = require('./embedded')
const brave = require('./brave')
const { precache } = require('../precache')
const { prepareReloadExtensions, WebUiReloader, LocalGatewayReloader, InternalTabReloader } = require('./reloaders')

// ensure single client at all times, and no overlap between init and destroy
let client
Expand Down Expand Up @@ -61,40 +62,54 @@ async function destroyIpfsClient (browser) {
}
}

function _isWebuiTab (url) {
const bundled = !url.startsWith('http') && url.includes('/webui/index.html#/')
const ipns = url.includes('/webui.ipfs.io/#/')
return bundled || ipns
}

function _isInternalTab (url, extensionOrigin) {
return url.startsWith(extensionOrigin)
}

async function _reloadIpfsClientDependents (browser, instance, opts) {
/**
* Reloads pages dependant on ipfs to be online
*
* @typedef {embedded|brave|external} Browser
* @param {Browser} browser
* @param {import('ipfs-http-client').default} instance
* @param {Object} opts
* @param {Array.[InternalTabReloader|LocalGatewayReloader|WebUiReloader]=} reloadExtensions
* @returns {void}
*/
async function _reloadIpfsClientDependents (
browser, instance, opts, reloadExtensions = [WebUiReloader, LocalGatewayReloader, InternalTabReloader]) {
// online || offline
if (browser.tabs && browser.tabs.query) {
const tabs = await browser.tabs.query({})
if (tabs) {
const extensionOrigin = browser.runtime.getURL('/')
tabs.forEach((tab) => {
// detect bundled webui in any of open tabs
if (_isWebuiTab(tab.url)) {
log(`reloading webui at ${tab.url}`)
browser.tabs.reload(tab.id)
} else if (_isInternalTab(tab.url, extensionOrigin)) {
log(`reloading internal extension page at ${tab.url}`)
browser.tabs.reload(tab.id)
}
})
try {
const reloadExtensionInstances = await prepareReloadExtensions(reloadExtensions, browser, log)
// the reload process is async, fire and forget.
reloadExtensionInstances.forEach(ext => ext.reload(tabs))
} catch (e) {
log('Failed to trigger reloaders')
}
}
}

// online only
if (client && instance && opts) {
// add important data to local ipfs repo for instant load
setTimeout(() => precache(instance, opts), 5000)
}
}

exports.initIpfsClient = initIpfsClient
exports.destroyIpfsClient = destroyIpfsClient
/**
* Reloads local gateway pages dependant on ipfs to be online
*
* @typedef {embedded|brave|external} Browser
* @param {Browser} browser
* @param {import('ipfs-http-client').default} instance
* @param {Object} opts
* @returns {void}
*/
function reloadIpfsClientOfflinePages (browser, instance, opts) {
_reloadIpfsClientDependents(browser, instance, opts, [LocalGatewayReloader])
}

module.exports = {
initIpfsClient,
destroyIpfsClient,
reloadIpfsClientOfflinePages
}
33 changes: 33 additions & 0 deletions add-on/src/lib/ipfs-client/reloaders/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
const { InternalTabReloader } = require('./internalTabReloader')
const { LocalGatewayReloader } = require('./localGatewayReloader')
const { WebUiReloader } = require('./webUiReloader')

/**
* Prepares extension by creating an instance and awaiting for init.
*
* @param {Array.[InternalTabReloader|LocalGatewayReloader|WebUiReloader]} extensions
* @param {Browser} browserInstance
* @param {Logger} loggerInstance
* @returns {Promise<Array.[InternalTabReloader|LocalGatewayReloader|WebUiReloader]>}
*/
function prepareReloadExtensions (extensions, browserInstance, loggerInstance) {
const reloadExtensions = Array.isArray(extensions) ? extensions : [extensions]
return Promise.all(reloadExtensions
.map(async Ext => {
try {
const ext = new Ext(browserInstance, loggerInstance)
await ext.init()
return ext
} catch (e) {
loggerInstance(`Extension Instance Failed to Initialize with error: ${e}. Extension: ${Ext}`)
}
})
)
}

module.exports = {
InternalTabReloader,
LocalGatewayReloader,
WebUiReloader,
prepareReloadExtensions
}
37 changes: 37 additions & 0 deletions add-on/src/lib/ipfs-client/reloaders/internalTabReloader.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
const { ReloaderBase } = require('./reloaderBase')

class InternalTabReloader extends ReloaderBase {
/**
* Setting up the extension origin.
*/
init () {
this.extensionOrigin = this._browserInstance.runtime.getURL('/')
this._log('InternalTabReloader Ready for use.')
}

/**
* Performs url validation for the tab. If tab is a WebUI tab.
*
* @param {Object} tab
* @param {string} tab.url
* @returns {boolean}
*/
validation ({ url }) {
return url.startsWith(this.extensionOrigin)
}

/**
* Returns message when reloading the tab.
*
* @param {Object} tab
* @param {string} tab.url
* @returns {string} message.
*/
message ({ url }) {
return `reloading internal extension page at ${url}`
}
}

module.exports = {
InternalTabReloader
}
50 changes: 50 additions & 0 deletions add-on/src/lib/ipfs-client/reloaders/localGatewayReloader.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
const { ReloaderBase } = require('./reloaderBase')

class LocalGatewayReloader extends ReloaderBase {
/**
* Fetching the customGatewayUrl from the local storage.
*/
async init () {
const { customGatewayUrl } = await this._browserInstance.storage.local.get('customGatewayUrl')
this.customGatewayUrl = customGatewayUrl
this._log('LocalGatewayReloader Ready for use.')
}

/**
* Performs url validation for the tab. If tab is loaded via local gateway.
*
* @param {Object} tab
* @param {string} tab.url
* @param {string} tab.url
* @returns {boolean}
*/
validation ({ url, title }) {
/**
* Check if the url is the local gateway url and if the title is contained within the url then it was not loaded.
* - This assumes that the title of most pages on the web will be set and hence when not reachable, the browser
* will set title to the url/host (both chrome and brave) and 'problem loading page' for firefox.
* - There is probability that this might be true in case the <title> tag is omitted, but worst case it only reloads
* those pages.
* - The benefit we get from this approach is the static nature of just observing the tabs in their current state
* which reduces the overhead of injecting content scripts to track urls that were loaded after the connection
* was offline, it may also need extra permissions to inject code on error pages.
*/
return url.startsWith(this.customGatewayUrl) &&
(url.includes(title) || title.toLowerCase() === 'problem loading page')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two nits:

  • startsWith does not cover subdomain gateways
  • browser vendors may change the title (in Firefox it will be different for non-english locale)
    • (loose idea, not sure how feasible) potential improvement would be to leverage onErrorOccurred from src/lib/ipfs-request.js and inside of it, add isRecoverableViaOnlineApi which would save failed requests to local gateway in a cache similar to errorInFlight. Then, you could check if url is present in that cache, removing the need for matching titles. More complicated, but future-proof :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @lidel

  • the check should be isIPFS.url || isIPFS.subdomain and not isIPFS.url && isIPFS.subdomain added that.
  • I'll create a separate issue for that, It was on my mind but this PR is large as it is, I'll work on that issue separately.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@whizzzkid mind linking to the created issue or creating it and commenting here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, #1097

Can you please assign this to me @SgtPooki/@lidel

}

/**
* Returns message when reloading the tab.
*
* @param {Object} tab
* @param {string} tab.url
* @returns {string} message.
*/
message ({ url }) {
return `reloading local gateway at ${url}`
}
}

module.exports = {
LocalGatewayReloader
}
53 changes: 53 additions & 0 deletions add-on/src/lib/ipfs-client/reloaders/reloaderBase.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
class ReloaderBase {
/**
* Constructor for reloader base class.
*
* @param {Browser} browser
* @param {Logger} log
*/
constructor (browser, log) {
if (!browser || !log) {
throw new Error('Instances of browser and logger are needed!')
}
this._browserInstance = browser
this._log = log
};

/**
* Initializes the instance.
*/
init () {
this._log('Initialized without additional config.')
}

/**
* To be implemented in child class.
*/
validation () {
throw new Error('Validation: Method Not Implemented')
}

/**
* To be implemented in child class.
*/
message () {
throw new Error('Message: Method Not Implemented')
}

/**
* Handles reload for all tabs.
* params {Array<tabs>}
*/
reload (tabs) {
tabs
.filter(tab => this.validation(tab))
.forEach(tab => {
this._log(this.message(tab))
this._browserInstance.tabs.reload(tab.id)
})
}
}

module.exports = {
ReloaderBase
}
30 changes: 30 additions & 0 deletions add-on/src/lib/ipfs-client/reloaders/webUiReloader.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
const { ReloaderBase } = require('./reloaderBase')

class WebUiReloader extends ReloaderBase {
/**
* Performs url validation for the tab. If tab is a WebUI tab.
*
* @param {Object} tab
* @returns {boolean}
*/
validation ({ url }) {
const bundled = !url.startsWith('http') && url.includes('/webui/index.html#/')
const ipns = url.includes('/webui.ipfs.io/#/')
return bundled || ipns
}
Comment on lines +10 to +14
Copy link
Member

@lidel lidel Sep 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: built-in webui at http://127.0.0.1:5001/webui/ returns redirect to http://127.0.0.1:5001/ipfs/{hardcoded-cid} (hardcoded-cid here)

It seems to be missing here, is it reloaded by some other means? (if not, we may want to add it)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This functionality was brought in from: https://github.com/ipfs/ipfs-companion/blob/main/add-on/src/lib/ipfs-client/index.js#L64-L68

But now that I think more about it, this will be handled in the localGatewayReloader check as isIPFS.url || isIPFS.subdomain returns true for http://127.0.0.1:5001/ipfs/bafybeibozpulxtpv5nhfa2ue3dcjx23ndh3gwr5vwllk7ptoyfwnfjjr4q hardcoded cid.


/**
* Returns message when reloading the tab.
*
* @param {Object} tab
* @param {string} tab.url
* @returns {string} message.
*/
message ({ url }) {
return `reloading webui at ${url}`
}
}

module.exports = {
WebUiReloader
}
3 changes: 2 additions & 1 deletion add-on/src/lib/ipfs-companion.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const { initState, offlinePeerCount } = require('./state')
const { createIpfsPathValidator, sameGateway, safeHostname } = require('./ipfs-path')
const createDnslinkResolver = require('./dnslink')
const { createRequestModifier } = require('./ipfs-request')
const { initIpfsClient, destroyIpfsClient } = require('./ipfs-client')
const { initIpfsClient, destroyIpfsClient, reloadIpfsClientOfflinePages } = require('./ipfs-client')
const { braveNodeType, useBraveEndpoint, releaseBraveEndpoint } = require('./ipfs-client/brave')
const { createIpfsImportHandler, formatImportDirectory, browserActionFilesCpImportCurrentTab } = require('./ipfs-import')
const createNotifier = require('./notifier')
Expand Down Expand Up @@ -595,6 +595,7 @@ module.exports = async function init () {
if (oldPeerCount === offlinePeerCount && newPeerCount > offlinePeerCount && !state.redirect) {
await browser.storage.local.set({ useCustomGateway: true })
await notify('notify_apiOnlineTitle', 'notify_apiOnlineAutomaticModeMsg')
reloadIpfsClientOfflinePages(browser, ipfs, state)
} else if (newPeerCount === offlinePeerCount && state.redirect) {
await browser.storage.local.set({ useCustomGateway: false })
await notify('notify_apiOfflineTitle', 'notify_apiOfflineAutomaticModeMsg')
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@
"webextension-polyfill": "0.7.0"
},
"engines": {
"node": ">=14.15.0",
"node": ">=14.15.0 <17",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for some reason latest LTS (18) fails to build this. I did not investigate, but added check to prevent this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. a lot of our packages and their dependencies are out of date, and there have been a lot of changes made w.r.t. esm support, so we've been in a tough spot. However, you can see our support goals at https://github.com/ipfs/ipfs-gui/tree/SgtPooki-version-support-definition#nodejs (We can link to main branch when ipfs/ipfs-gui#97 gets merged)

"npm": ">=6.14.0"
}
}
Loading