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

Bug 1687491 - Dispatch all external API calls #40

Merged
merged 11 commits into from
Feb 8, 2021
8 changes: 6 additions & 2 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,12 @@
"files": "*.ts",
"extends": [
"plugin:@typescript-eslint/eslint-recommended",
"plugin:@typescript-eslint/recommended"
]
"plugin:@typescript-eslint/recommended",
"plugin:@typescript-eslint/recommended-requiring-type-checking"
],
"parserOptions": {
"project": ["./tsconfig.json"]
}
}
]
}
8 changes: 1 addition & 7 deletions bin/qt-js-check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,8 @@ xvfb-run python main.py &> qml.log &

sleep 10

if grep -q "Some Javascript error occured" "qml.log"; then
if grep -q "Error executing task:" "qml.log"; then
echo "Failed to initialize Glean in QML! See more logs below."
cat qml.log
exit 1
fi

if ! grep -q "Called Glean.initialize" "qml.log"; then
echo "Unable to initialize Glean in QML! See more logs below."
cat qml.log
exit 1
fi
6 changes: 1 addition & 5 deletions samples/qt-qml-app/main.qml
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,6 @@ Rectangle {

Component.onCompleted: {
// Initialize Glean when the application starts.
Glean.Glean.initialize("qt-qml-app", true)
// Note: If you change this console messages you should change
// the console message verifications on `bin/qt-js-check.sh.
.then(() => console.log("Called Glean.initialize succesfully."))
.catch(err => console.error(`Some Javascript error occured.\n${err}`));
Glean.Glean.initialize("qt-qml-app", true);
brizental marked this conversation as resolved.
Show resolved Hide resolved
}
}
20 changes: 5 additions & 15 deletions samples/web-extension/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,20 @@ import Glean from "glean";
import { samplePing } from "./generatedPings";
import { webExtStarted, popupOpened } from "./generatedMetrics";

// TODO: Do not wait for Glean to be initialized before recording metrics
// once Bug 1687491 is resolved.
Glean.initialize("web-extension", true)
.then(() => {
console.log("Glean has been succesfully initialized.");
webExtStarted.set()
.then(() => console.log("`webext-installed` was succesfully set."));
});

Glean.initialize("web-extension", true);
webExtStarted.set();

// Listen for messages from the popup.
browser.runtime.onMessage.addListener(msg => {
console.log(`New message received! ${msg}`);

if (msg === "popup-opened") {
popupOpened.add()
.then(() => console.log("`popup-opened` was succesfully added."));
popupOpened.add();
}

if (msg === "send-ping") {
samplePing.submit()
.then(wasSubmitted => {
console.log(
`Attempted to send ping "${samplePing.name}". Was the ping sent? ${wasSubmitted}`);
});
samplePing.submit();
}
});

173 changes: 163 additions & 10 deletions src/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import { generateUUIDv4 } from "utils";

// The possible states a dispatcher instance can be in.
export const enum DispatcherState {
// The dispatcher has not been initialized yet.
Expand Down Expand Up @@ -34,12 +36,29 @@ type Task = () => Promise<void>;

// An executable command.
type Command = {
testId?: string,
task: Task,
command: Commands.Task,
} | {
command: Exclude<Commands, Commands.Task>,
};

/**
* An observer of the commands being executed by the dispatcher.
*/
class DispatcherObserver {
constructor(private cb: (command: Command) => void) {}

/**
* Updates an observer when the dispatcher finishes executing a command.
*
* @param command The command that was just executed by the dispatcher.
*/
update(command: Command): void {
this.cb(command);
}
}

/**
* A task dispatcher for async tasks.
*
Expand All @@ -55,11 +74,56 @@ class Dispatcher {
// This is `undefined` in case there is no ongoing execution of tasks.
private currentJob?: Promise<void>;

// Observers that are notified about every executed command in this dispacther.
//
// This is private, because we only expect `testLaunch` to attach observers as of yet.
private observers: DispatcherObserver[];

constructor(readonly maxPreInitQueueSize = 100) {
this.observers = [];
this.queue = [];
this.state = DispatcherState.Uninitialized;
}

/**
* Attaches an observer that will be notified about state changes on the Dispatcher.
*
* # Note
*
* This is a private function because only the `testLaunch` function
* is expected to watch the state of the Dispatcher as of yet.
*
* @param observer The observer to attach.
*/
private attachObserver(observer: DispatcherObserver): void {
this.observers.push(observer);
}

/**
* Un-attaches an observer that will be notified about state changes on the Dispatcher.
*
* # Note
*
* This is a private function because only the `testLaunch` function
* is expected to watch the state of the Dispatcher as of yet.
*
* @param observer The observer to attach.
*/
private unattachObserver(observer: DispatcherObserver): void {
this.observers = this.observers.filter(curr => observer !== curr);
}

/**
* Notify any currently attached observer that a new command was executed.
*
* @param command The command to notify about
*/
private notifyObservers(command: Command): void {
for (const observer of this.observers) {
observer.update(command);
}
}

/**
* Gets the oldest command added to the queue.
*
Expand Down Expand Up @@ -91,13 +155,16 @@ class Dispatcher {
switch(nextCommand.command) {
case(Commands.Stop):
this.state = DispatcherState.Stopped;
this.notifyObservers(nextCommand);
return;
case(Commands.Clear):
this.queue = [];
this.state = DispatcherState.Stopped;
this.notifyObservers(nextCommand);
return;
case(Commands.Task):
await this.executeTask(nextCommand.task);
this.notifyObservers(nextCommand);
nextCommand = this.getNextCommand();
}
}
Expand All @@ -118,12 +185,19 @@ class Dispatcher {
// That should be avoided as much as possible,
// because it will cause a deadlock in case you wait inside a task
// that was launched inside another task.
this.currentJob.then(() => {
this.currentJob = undefined;
if (this.state === DispatcherState.Processing) {
this.state = DispatcherState.Idle;
}
});
this.currentJob
.then(() => {
this.currentJob = undefined;
if (this.state === DispatcherState.Processing) {
this.state = DispatcherState.Idle;
}
})
.catch(error => {
console.error(
"IMPOSSIBLE: Something went wrong while the dispatcher was executing the tasks queue.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Bets on when we'll see this message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no bets, unless we change the executeTask to not catch errors or use the observers for something else. right now it should be impossible for this to error. i only added the catch here because the linter was complaining :D

error
);
});
}
}

Expand All @@ -139,12 +213,14 @@ class Dispatcher {
* @param command The command to enqueue.
* @param priorityTask Whether or not this task is a priority task
* and should be enqueued at the front of the queue.
*
* @returns Wheter or not the task was queued.
*/
private launchInternal(command: Command, priorityTask = false): void {
private launchInternal(command: Command, priorityTask = false): boolean {
if (!priorityTask && this.state === DispatcherState.Uninitialized) {
if (this.queue.length >= this.maxPreInitQueueSize) {
console.warn("Unable to enqueue task, pre init queue is full.");
return;
return false;
}
}

Expand All @@ -155,6 +231,8 @@ class Dispatcher {
}

this.triggerExecution();

return true;
}

/**
Expand All @@ -180,13 +258,22 @@ class Dispatcher {
* Flushes the tasks enqueued while the dispatcher was uninitialized.
*
* This is a no-op in case the dispatcher is not in an uninitialized state.
*
* @param task Optional task to execute before any of the tasks enqueued prior to init.
*/
flushInit(): void {
flushInit(task?: Task): void {
if (this.state !== DispatcherState.Uninitialized) {
console.warn("Attempted to initialize the Dispatcher, but it is already initialized. Ignoring.");
return;
}

if (task) {
this.launchInternal({
task,
command: Commands.Task
}, true);
}

this.state = DispatcherState.Idle;
this.triggerExecution();
}
Expand Down Expand Up @@ -238,7 +325,7 @@ class Dispatcher {
*
* Returns a promise that resolves once the current task execution in finished.
*
* Use this with caution.
* Use this with caution.
* If called inside a task launched inside another task, it will cause a deadlock.
*
* @returns The promise.
Expand All @@ -260,6 +347,72 @@ class Dispatcher {
await this.testBlockOnQueue();
this.state = DispatcherState.Uninitialized;
}

/**
* Launches a task in test mode.
*
* # Note
*
* This function will resume the execution of tasks if the dispatcher was stopped
* and return the dispatcher back to an idle state.
*
* This is important in order not to hang forever in case the dispatcher is stopped.
*
* @param task The task to launch.
*
* @returns A promise which only resolves once the task is done being executed
* or is guaranteed to not be executed ever i.e. if the queue gets cleared.
* This promise is rejected if the dispatcher takes more than 1s
* to execute the current task i.e. if the dispatcher is uninitialized.
*/
testLaunch(task: Task): Promise<void> {
brizental marked this conversation as resolved.
Show resolved Hide resolved
const testId = generateUUIDv4();
console.info("Launching a test task.", testId);

this.resume();
const wasLaunched = this.launchInternal({
testId,
task,
command: Commands.Task
});

if (!wasLaunched) {
return Promise.reject();
}

// This promise will resolve:
//
// - If the dispatcher gets a Clear command;
// - If a task with `testId` is executed;
//
// This promise will reject if:
//
// - If we wait for this task to be execute for more than 1s.
// This is to attend to the case where the dispatcher is Stopped
// and the task takes to long to be executed.
return new Promise((resolve, reject) => {
const observer = new DispatcherObserver((command: Command) => {
const isCurrentTask = (command.command === Commands.Task && command.testId === testId);
if (isCurrentTask || command.command === Commands.Clear) {
this.unattachObserver(observer);
clearTimeout(timeout);
resolve();
}
});

const timeout = setTimeout(() => {
console.error(
`Test task ${testId} took to long to execute.`,
"Please check if the dispatcher was initialized and is not stopped.",
"Bailing out."
);
this.unattachObserver(observer);
reject();
}, 1000);

this.attachObserver(observer);
});
}
}

export default Dispatcher;
Loading