Wrapped errors from sendMessage should include the original error name, stacktrace

[lguychard]

Errors returned by onMessage listeners are currently converted to JSON:

webextension-polyfill/src/browser-polyfill.js

Lines 410 to 423 in 5f7a845

	// Send a JSON representation of the error if the rejected value
	// is an instance of error, or the object itself otherwise.
	let message;
	if (error && (error instanceof Error ||
	typeof error.message === "string")) {
	message = error.message;
	} else {
	message = "An unexpected error occurred";
	}
	sendResponse({
	__mozWebExtensionPolyfillReject__: true,
	message,
	});

At present, only the message property of the original error is sent in the response. In the case where the rejected value is an Error instance, it would be nice to include other structured clone serializable properties from the original error, such as name and stacktrace.

[Rob--W]

This could be useful for debugging, but adds complexity and overhead to the implementation. Most users probably don't need this functionality.

Therefore we'll close this bug.

[felixfbecker]

but adds complexity and overhead to the implementation

If we just copy over two additional properties that every Error has, name and stack, would that still be too much complexity and overhead? That should be only
two lines of code.

Most users probably don't need this functionality.

I'd actually argue the opposite extreme: Wanting to see the stack traces of errors is not a niche use case, but rather something 100% of the users of the polyfill would want.

[Rob--W]

If we just copy over two additional properties that every Error has, name and stack, would that still be too much complexity and overhead? That should be only two lines of code.

To implement this, we would need to special-case errors and add the stack here:

webextension-polyfill/src/browser-polyfill.js

Line 413 in 5f7a845

if (error && (error instanceof Error ||

Then at the other side, we would have to reconstruct the (typed) error object at

webextension-polyfill/src/browser-polyfill.js

Line 457 in 5f7a845

reject(new Error(reply.message));

.

In terms of code, it is not very difficult, but nonetheless it's more than nothing.

Wanting to see the stack traces of errors is not a niche use case, but rather something 100% of the users of the polyfill would want.

I'm sympathic to the idea of stack traces (I first-hand experience the pain of some bugs in missing/incorrect stack traces in Firefox during some unit tests), but I still maintain that it's mainly a development feature, and not something that can or should be relied upon for control flow in the extension (chrome.runtime.lastError doesn't have anything else besides .message, and the stack is not even documented on MDN).

But if rejected messages are expected to be rare, then the overhead of supporting stack traces are not as bad as I thought. I may consider re-opening the feature request (and review+accept a pull request), if @rpl agrees to.

[rpl]

@Rob--W sure, I'm also ok with re-opening this and marking it as "contrib: welcome".

[lguychard]

I'll happily send up a PR for this.

[franciscolourenco]

Stack traces are needed. @lguychard are you actively working on this?

[franciscolourenco]

@lguychard any news? Thanks!

[philipp-tailor]

Then at the other side, we would have to reconstruct the (typed) error object at

webextension-polyfill/src/browser-polyfill.js

Line 457 in 5f7a845

reject(new Error(reply.message));

.

+1 for the idea of converting error objects with a message as Error instances!

I'm contributing to an extension where rejections are shown as UnhandledRejection: Non-Error promise rejection captured with keys: message in sentry because the argument of the rejection is not an instance of Error. That makes it unnecessary hard to maintain / monitor the extension.

If this change would be welcome, I could be open to contributing it.

[philipp-tailor]

Actually I'm realizing that the change I proposed was issue #257, which has been implemented with #293. Thank you! 👏🏼

[fregante]

It might be useful to use serialize-errors, if it matches Firefox’ behavior: https://github.com/sindresorhus/serialize-error