Bug 220820 - [JSC] JSPromise should not propagate TerminatedExecutionError
Summary: [JSC] JSPromise should not propagate TerminatedExecutionError
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-01-21 13:11 PST by Yusuke Suzuki
Modified: 2021-01-22 10:59 PST (History)
7 users (show)

See Also:


Attachments
Patch (4.02 KB, patch)
2021-01-21 13:12 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (25.92 KB, patch)
2021-01-21 15:11 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2021-01-21 13:11:26 PST
[JSC] JSPromise should not propagate TerminatedExecutionError
Comment 1 Yusuke Suzuki 2021-01-21 13:12:42 PST
Created attachment 418077 [details]
Patch
Comment 2 Yusuke Suzuki 2021-01-21 13:12:45 PST
<rdar://problem/72929399>
Comment 3 Keith Miller 2021-01-21 13:13:53 PST
r=me
Comment 4 Yusuke Suzuki 2021-01-21 15:11:20 PST
Created attachment 418091 [details]
Patch
Comment 5 Mark Lam 2021-01-21 15:21:10 PST
Comment on attachment 418091 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=418091&action=review

> Source/JavaScriptCore/API/JSAPIGlobalObject.mm:167
> +    // FIXME: We could have error since any JS call can throw stack-overflow errors.
> +    // https://bugs.webkit.org/show_bug.cgi?id=203402
> +    promise->reject(globalObject, createError(globalObject, result.error()));

Shouldn't you only reject the promise if an exception actually exists?
Comment 6 Mark Lam 2021-01-21 15:33:00 PST
Comment on attachment 418091 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=418091&action=review

r=me

>> Source/JavaScriptCore/API/JSAPIGlobalObject.mm:167
>> +    promise->reject(globalObject, createError(globalObject, result.error()));
> 
> Shouldn't you only reject the promise if an exception actually exists?

Sorry, I misread this.  Ignore me.
Comment 7 Yusuke Suzuki 2021-01-21 18:20:41 PST
Comment on attachment 418091 [details]
Patch

Thanks!
Comment 8 EWS 2021-01-21 18:51:32 PST
Committed r271731: <https://trac.webkit.org/changeset/271731>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 418091 [details].