Bug 1441333: Part 6 - Use caller location in error reports for StrongPromise errors. r=zombie
We currently report a useful location in error reports when extensions fail to
resolve a promise or call a response callback, but in some slightly
less-than-ideal ways. We currently generate a complete stack and parse its
string value (which is expensive), and then report the caller location as part
of the message, rather than as the error's location and stack.
This patch changes that behavior to store a single SavedStack frame, and to
properly report that as the location of the error.
MozReview-Commit-ID: Jmtf4C1O6pW
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -85,35 +85,43 @@ function injectAPI(source, dest) {
* A finalization witness helper that wraps a sendMessage response and
* guarantees to either get the promise resolved, or rejected when the
* wrapped promise goes out of scope.
*
* Holding a reference to a returned StrongPromise doesn't prevent the
* wrapped promise from being garbage collected.
*/
const StrongPromise = {
+ locations: new Map(),
+
wrap(promise, channelId, location) {
return new Promise((resolve, reject) => {
- const tag = `${channelId}|${location}`;
- const witness = finalizationService.make("extensions-sendMessage-witness", tag);
+ this.locations.set(channelId, location);
+
+ const witness = finalizationService.make("extensions-sendMessage-witness", channelId);
promise.then(value => {
+ this.locations.delete(channelId);
witness.forget();
resolve(value);
}, error => {
+ this.locations.delete(channelId);
witness.forget();
reject(error);
});
});
},
- observe(subject, topic, tag) {
- const pos = tag.indexOf("|");
- const channel = Number(tag.substr(0, pos));
- const location = tag.substr(pos + 1);
- const message = `Promised response from onMessage listener at ${location} went out of scope`;
- MessageChannel.abortChannel(channel, {message});
+ observe(subject, topic, channelId) {
+ channelId = Number(channelId);
+ let location = this.locations.get(channelId);
+ this.locations.delete(channelId);
+
+ const message = `Promised response from onMessage listener went out of scope`;
+ const error = ChromeUtils.createError(message, location);
+ error.mozWebExtLocation = location;
+ MessageChannel.abortChannel(channelId, error);
},
};
Services.obs.addObserver(StrongPromise, "extensions-sendMessage-witness");
/**
* Abstraction for a Port object in the extension API.
*
* @param {BaseContext} context The context that owns this port.
@@ -391,17 +399,17 @@ class Messenger {
sendNativeMessage(messageManager, msg, recipient, responseCallback) {
msg = NativeApp.encodeMessage(this.context, msg);
return this.sendMessage(messageManager, msg, recipient, responseCallback);
}
_onMessage(name, filter) {
return new EventManager(this.context, name, fire => {
- const [location] = new this.context.cloneScope.Error().stack.split("\n", 1);
+ const caller = this.context.getCaller();
let listener = {
messageFilterPermissive: this.optionalFilter,
messageFilterStrict: this.filter,
filterMessage: (sender, recipient) => {
// Exclude messages coming from content scripts for the devtools extension contexts
// (See Bug 1383310).
@@ -435,19 +443,19 @@ class Messenger {
sendResponse = Cu.exportFunction(sendResponse, this.context.cloneScope);
// Note: We intentionally do not use runSafe here so that any
// errors are propagated to the message sender.
let result = fire.raw(message, sender, sendResponse);
message = null;
if (result instanceof this.context.cloneScope.Promise) {
- return StrongPromise.wrap(result, channelId, location);
+ return StrongPromise.wrap(result, channelId, caller);
} else if (result === true) {
- return StrongPromise.wrap(promise, channelId, location);
+ return StrongPromise.wrap(promise, channelId, caller);
}
return response;
},
};
MessageChannel.addListener(this.messageManagers, "Extension:Message", listener);
return () => {
MessageChannel.removeListener(this.messageManagers, "Extension:Message", listener);
--- a/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js
@@ -151,20 +151,33 @@ add_task(async function sendMessageRespo
browser.test.onMessage.addListener(msg => {
browser.runtime.sendMessage(msg)
.then(
response => {
if (response) {
browser.test.log(`Got response: ${response}`);
browser.test.sendMessage(response);
}
- }, ({message}) => {
+ }, error => {
+ browser.test.assertEq(
+ "Promised response from onMessage listener went out of scope",
+ error.message,
+ `Promise rejected with the correct error message`);
+
browser.test.assertTrue(
- /at background@moz-extension:\/\/[\w-]+\/%7B[\w-]+%7D\.js:4:\d went out/.test(message),
- `Promise rejected with the correct error message: ${message}`);
+ /^moz-extension:\/\/[\w-]+\/%7B[\w-]+%7D\.js/.test(error.fileName),
+ `Promise rejected with the correct error filename: ${error.fileName}`);
+
+ browser.test.assertEq(
+ 4, error.lineNumber,
+ `Promise rejected with the correct error line number`);
+
+ browser.test.assertTrue(
+ /moz-extension:\/\/[\w-]+\/%7B[\w-]+%7D\.js:4/.test(error.stack),
+ `Promise rejected with the correct error stack: ${error.stack}`);
browser.test.sendMessage("rejected");
});
});
browser.test.sendMessage("ready");
}
let extension = ExtensionTestUtils.loadExtension({
background,