JS Exception Support #102

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

Jump to bottom

Merged

kateinoigakukun merged 5 commits into main from katei/exception

Oct 23, 2020

Merged

JS Exception Support #102

kateinoigakukun merged 5 commits into main from katei/exception

Oct 23, 2020

Conversation

@kateinoigakukun

Copy link

Member

@kateinoigakukun kateinoigakukun commented Oct 22, 2020 •

edited

Loading

Currently, there is no way to catch exceptions thrown by JavaScript functions using JavaScriptKit.

This PR introduces new two APIs to catch exceptions.

JSFunction.throws
JSObject.throwing

JSFunction.throws converts JSFunction into throwable function. JSObject.throwing provides throwing method call API capturing this context.

These two APIs have different names to avoid overload ambiguity.

e.g.

// Throwing function call
let fn1: JSFunction = ...
try fn1.throws()
// Throwing constructor call
let Animal: JSFunction = ...
let cat1 = Animal.throws.new()
// Throwing method call
let obj1: JSObject = ...
obj1.throwing.method1!()

kateinoigakukun added 2 commits

October 22, 2020 23:45

@kateinoigakukun


 Implement JSThrowingFunction

9db9885

@kateinoigakukun


 Add test cases

dd0d2f5

@github-actions

Copy link

github-actions bot commented Oct 22, 2020 •

edited

Loading

Time Change: +75.5ms (0%)

Total Time: 10,698ms

Test name	Duration	Change
Serialization/Write JavaScript string directly	182.5ms	-10.5ms (5%)	✅

i️ View Unchanged

Test name	Duration	Change
Serialization/Write JavaScript number directly	183.5ms	+8.5ms (4%)
Serialization/Swift Int to JavaScript	3,505ms	-45ms (1%)
Serialization/Swift String to JavaScript	3,554.25ms	-3.25ms (0%)
Object heap/Increment and decrement RC	3,272.75ms	+125.75ms (3%)

_{performance-action}

@kateinoigakukun kateinoigakukun marked this pull request as ready for review

October 22, 2020 15:55

@kateinoigakukun


 Bump Runtime version

0ac5425

MaxDesiatov

MaxDesiatov approved these changes

Oct 22, 2020

View reviewed changes

Copy link

Member

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

LGTM, awesome work!

@kateinoigakukun kateinoigakukun requested a review from j-f1

October 23, 2020 05:01

j-f1

j-f1 reviewed

Oct 23, 2020

View reviewed changes

Runtime/src/index.ts Outdated Show resolved Hide resolved

Runtime/src/index.ts Show resolved Hide resolved

Runtime/src/index.ts Outdated

let result: any

try {

result = Reflect.construct(obj, decodeValues(argv, argc));

if (typeof result != "object")

Copy link

Member

@j-f1 j-f1 Oct 23, 2020

Choose a reason for hiding this comment

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

I’m pretty sure new can’t return a non-object. If you try to return a non-object in a constructor, it will use the this value instead.

See the spec here, where step 12 is the most relevant.

kateinoigakukun added 2 commits

October 23, 2020 23:08

@kateinoigakukun


 Apply npx prettier --write Runtime/src/index.ts

5f9f56c

@kateinoigakukun


 Remove constructor result type check

@kateinoigakukun

Copy link

Member Author

kateinoigakukun commented Oct 23, 2020 •

edited

Loading

@j-f1 @MaxDesiatov What do you think about this API? I think there may be better interface.

@MaxDesiatov

Copy link

Member

MaxDesiatov commented Oct 23, 2020

If there's a better API for it, I can't come up with it...

j-f1

j-f1 approved these changes

Oct 23, 2020

View reviewed changes

Copy link

Member

@j-f1 j-f1 left a comment

Choose a reason for hiding this comment

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

LGTM! We’re not at 1.0 yet so we can continue to iterate if necessary.

@kateinoigakukun kateinoigakukun merged commit 6f31fc3 into main

Oct 23, 2020

@kateinoigakukun kateinoigakukun deleted the katei/exception branch

October 23, 2020 16:41

@MaxDesiatov MaxDesiatov mentioned this pull request

Nov 4, 2020

Support JS errors #37

Closed

Labels

None yet

4 participants

@kateinoigakukun @MaxDesiatov @j-f1

Uh oh!

JS Exception Support #102

JS Exception Support #102

Uh oh!

Conversation

@kateinoigakukun kateinoigakukun commented Oct 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

@j-f1 j-f1 Oct 23, 2020

Choose a reason for hiding this comment

Uh oh!

kateinoigakukun commented Oct 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaxDesiatov commented Oct 23, 2020

Uh oh!

@j-f1 j-f1 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

@kateinoigakukun kateinoigakukun commented Oct 22, 2020 •

edited

Loading

github-actions bot commented Oct 22, 2020 •

edited

Loading

kateinoigakukun commented Oct 23, 2020 •

edited

Loading