WebPlatformForEmbedded / WPEWebKit

WPE WebKit port (downstream)
213 stars 136 forks source link

[WPE-2.38] Missing line/column/sourceURL in Error objects in JS #1198

Closed piotr-marcinkowski-red closed 11 months ago

piotr-marcinkowski-red commented 11 months ago

When inspecting Error objects in webinspector it was found that the following properties were not attached: line, column, sourceURL. In WPE 2.22 these properties are present and in WPE 2.38 they are missing.

During the investigation it was confirmed that:

These properties are used in one of the test suites which expects them to be attached to Error objects. Since it collects them in a loop over properties it requires these properties to be enumerable.

The following pull request was created that adds the missing properties in Error instances. See the description there for more details on the implementation. https://github.com/LibertyGlobal/WPEWebKit/pull/316

The necessary changes were introduced to JavaScriptCore so it would be justified to first review this modification with great care before deciding to merge it. There is a chance an alternative solution exists which doesn't need changes in such crucial part of code.

piotr-marcinkowski-red commented 11 months ago

image

pgorszkowski-igalia commented 11 months ago

Making line, column and sourceURL as not enumerable was done in: https://github.com/WebKit/WebKit/commit/330739e633e1fc57394ddfec6b2315b7ca507da9 (Non-standard Error properties should not be enumerable)

pgorszkowski-igalia commented 11 months ago

Probably, changing non-enumerable to enumerable and setting sourceURL from empty to "about:blank" can be done from the JS code, e.g.:

class MyError extends Error {
    constructor(message) {
        super(message); //call superclass constructor

        Object.defineProperty(this, "line", {
        enumerable: true,
        configurable: false,
        writable: true
    });

    if (this.sourceURL === null || this.sourceURL === undefined || (this.sourceURL !== null && this.sourceURL !== undefined && this.sourceURL === ''))
        this.sourceURL = "about:blank";
    }
}

@piotr-marcinkowski-red: Please let me know does it work for you and it is feasible.

pgorszkowski-igalia commented 11 months ago

@piotr-marcinkowski-red: can we close this ticket?

piotr-marcinkowski-red commented 11 months ago

@pgorszkowski-igalia Yes, closing it