Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Attempt to make frontend-to-addon comms more robust #1102

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
69 changes: 46 additions & 23 deletions testpilot/frontend/static-src/app/main.js
Expand Up @@ -42,39 +42,62 @@ app.extend({
app.me = new Me();
app.experiments = new ExperimentsCollection();

Promise.all([
app.me.fetch(),
app.experiments.fetch()
]).then(() => {
app.me.updateEnabledExperiments(app.experiments);
app.pageManager = new PageManager({
pageContainer: document.querySelector('[data-hook=page-container]')
});

app.pageManager = new PageManager({
pageContainer: document.querySelector('[data-hook=page-container]')
});
if (!app.router.history.started()) {
Copy link
Contributor Author

@lmorchard lmorchard Jul 19, 2016

Choose a reason for hiding this comment

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

Also, moving page manager & router init outside of the experiments fetch means we can show the error page.

Side effect will be that the page will start to render before the API actually returns results, so typically a sub-second lag until filling in with experiments. Previously, the whole page would just stay white until the experiments came in.

app.router.history.start();
// HACK for Issue #124 - sometimes popstate doesn't fire on navigation,
// but pageshow does. But, we just want to know if the URL changed.
addEventListener('pageshow', app.router.history.checkUrl, false);
}

if (!app.router.history.started()) {
app.router.history.start();
// HACK for Issue #124 - sometimes popstate doesn't fire on navigation,
// but pageshow does. But, we just want to know if the URL changed.
addEventListener('pageshow', app.router.history.checkUrl, false);
}
app.experiments.fetch().then(() => app.me.fetch()).then(() => {
app.me.updateEnabledExperiments(app.experiments);
}).catch((err) => {
// for now, log the error in the console & do nothing in the UI
console && console.error(err); // eslint-disable-line no-console
app.router.redirectTo('error');
});
},

// Send webChannel message to addon, use a Promise to wait for the answer.
waitForMessage(type, data, timeout = 10000) {
waitForMessage(type, data, timeout = 5000, retries = 3) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW, reduced timeout to 5 seconds from 10 - because the retries should actually lead to a total timeout of 15 seconds before rejection.

return new Promise((resolve, reject) => {
const rejectTimer = setTimeout(() => {
reject('waitForMessage timeout: ' + type);
}, timeout);
this.once('webChannel:' + type + '-result', (result) => {
clearTimeout(rejectTimer);
let attempts = 0;
let retryTimer;

// On timeout, try again or reject with an error if out of retries.
const timeoutHandler = () => {
if (attempts++ < retries) {
attemptFn(); // eslint-disable-line no-use-before-define
} else {
reject('waitForMessage timeout: ' + type);
}
};

// On message response, resolve the promise with the result.
const resultHandler = (result) => {
clearTimeout(retryTimer);
resolve(result);
});
this.webChannel.sendMessage(type, data);
};

const attemptFn = () => {
// Un-register the event handler before registering it, in case this is
// a retry. We don't want to accidentally handle it multiple times.
const resultMessageName = 'webChannel:' + type + '-result';
this.off(resultMessageName, resultHandler);
this.once(resultMessageName, resultHandler);

// Try sending the message.
this.webChannel.sendMessage(type, data);

// Set up a timer to trigger retry / reject
retryTimer = setTimeout(timeoutHandler, timeout);
};

// Finally, kick off our first attempt.
attemptFn();
});
},

Expand Down