pkgs/jni: Support Throwing Java Exceptions in Interface Impl. #1211

Anikate-De · 2024-06-17T12:21:57Z

Items in this PR:

Support throwing Java exceptions in interface implementation #1209
Add throw Java exceptions on the same thread test case to pkgs/jnigen/test/simple_package_test

I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

See our contributor guide for general expectations for PRs.
Larger or significant changes should be discussed in an issue before creating a PR.
Contributions to our repos should follow the Dart style guide and use dart format.
Most changes should add an entry to the changelog and may need to rev the pubspec package version.
Changes to packages require corresponding tests.

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

Anikate-De · 2024-06-17T12:26:58Z

To regenerate the bindings for jni, run the script in tool/generate_ffi_bindings.dart. And after changing the code generation in jnigen, you can generate all the bindings again using tool/regenerate_all_bindings.dart (in jnigen).

I haven't used the jnigen regeneration script yet as I was using a very particular use case of ok_http and manually regenerating the bindings when needed.

I will soon shift to a more general example of throwing exceptions in interface implementation to test this PR overall, and I will use tool/regenerate_all_bindings.dart then (thanks :))

Anikate-De · 2024-06-17T21:54:36Z

Thanks for all the help! @HosseinYousefi

I temporarily edited the kotlin_plugin example within jnigen to demonstrate (and test) that this works. It has a very crude interface Divider. And a class DividerUser that catches any java.lang.ArithmeticExceptions thrown by the former.
We then deliberately throw the same, and expect a 0 as a result, since it will be caught in the Kotlin code.

No more java.lang.reflect.UndeclaredThrowableException!

If you feel the logical contents of this PR are satisfactory, then I can revert the last commit (Editing the jnigen example), and then get it ready for review.

pkgs/jni/src/dartjni.c

pkgs/jnigen/lib/src/bindings/dart_generator.dart

HosseinYousefi · 2024-06-18T06:21:25Z

Great job, @Anikate-De!

Yes, revert the temp commit and add the test to test/simple_package in jnigen. We currently cram all the usecases there. There are other tests for the interface implementation there as well.

github-actions · 2024-06-19T09:25:45Z

PR Health

Coverage ⚠️

Details

File	Coverage
pkgs/jni/lib/src/jni.dart	💔 Not covered
pkgs/jni/lib/src/jprimitives.dart	💔 Not covered
pkgs/jni/lib/src/third_party/jni_bindings_generated.dart	💔 Not covered
pkgs/jnigen/lib/src/bindings/dart_generator.dart	💔 Not covered

This check for test coverage is informational (issues shown here will not fail the PR).

This check can be disabled by tagging the PR with skip-coverage-check.

License Headers ⚠️

Details

// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

Files
pkgs/jni/lib/src/third_party/jni_bindings_generated.dart

All source files should start with a license header.

Unrelated files missing license headers

Files
pkgs/ffi/example/main.dart
pkgs/ffigen/example/libclang-example/generated_bindings.dart
pkgs/ffigen/example/shared_bindings/generate.dart
pkgs/ffigen/example/shared_bindings/lib/generated/a_gen.dart
pkgs/ffigen/example/shared_bindings/lib/generated/a_shared_b_gen.dart
pkgs/ffigen/example/shared_bindings/lib/generated/base_gen.dart
pkgs/ffigen/example/simple/generated_bindings.dart
pkgs/ffigen/lib/src/config_provider/config_spec.dart
pkgs/ffigen/lib/src/header_parser/clang_bindings/clang_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_decl_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_symbol_address_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_type_name_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_reserved_keyword_collision_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_comment_markup_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_dart_handle_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_enum_int_mimic_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_forward_decl_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_functions_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_imported_types_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_native_func_typedef_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_opaque_dependencies_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_packed_structs_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_regress_384_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_struct_fptr_fields_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_typedef_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_unions_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_varargs_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_cjson_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_libclang_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_sqlite_bindings.dart
pkgs/ffigen/test/native_test/_expected_native_test_bindings.dart
pkgs/jni/lib/src/lang/jcharacter.dart
pkgs/jni/lib/src/third_party/generated_bindings.dart
pkgs/jni/lib/src/third_party/global_env_extensions.dart
pkgs/jnigen/android_test_runner/lib/main.dart
pkgs/jnigen/example/in_app_java/lib/android_utils.dart
pkgs/jnigen/example/kotlin_plugin/example/lib/main.dart
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_bindings.dart
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_plugin.dart
pkgs/jnigen/example/pdfbox_plugin/lib/pdfbox_plugin.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocument.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocumentInformation.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/_package.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/text/PDFTextStripper.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/text/_package.dart
pkgs/jnigen/lib/src/bindings/descriptor.dart
pkgs/jnigen/lib/src/elements/elements.g.dart
pkgs/jnigen/test/jackson_core_test/third_party/bindings/com/fasterxml/jackson/core/_package.dart
pkgs/jnigen/tool/command_runner.dart
pkgs/native_assets_cli/lib/src/api/builder.dart
pkgs/native_assets_cli/lib/src/api/linker.dart
pkgs/native_assets_cli/test/model/checksum_test.dart
pkgs/swiftgen/swift2objc/lib/src/ast/ast.dart
pkgs/swiftgen/swift2objc/lib/src/ast/declarations/built_in/built_in_declaration.dart

This check can be disabled by tagging the PR with skip-license-check.

Package publish validation ✔️

Details

Package	Version	Status
package:ffi	2.1.2	already published at pub.dev
package:ffigen	13.0.0-wip	WIP (no publish necessary)
package:jni	0.9.3-wip	WIP (no publish necessary)
package:jnigen	0.9.3-wip	WIP (no publish necessary)
package:native_assets_cli	0.6.1-wip	WIP (no publish necessary)
package:objective_c	1.1.0-wip	WIP (no publish necessary)
package:swiftgen	0.0.1-wip	WIP (no publish necessary)

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

pkgs/jni/lib/src/jni.dart

...simple_package_test/java/com/github/dart_lang/jnigen/interfaces/StringConverterConsumer.java

pkgs/jnigen/test/simple_package_test/runtime_test_registrant.dart

HosseinYousefi · 2024-06-19T10:05:05Z

Also please add a changelog entry to jnigen about this.

Anikate-De · 2024-06-20T04:30:29Z

Bumped up version to 0.9.3

Anikate-De · 2024-06-20T04:34:15Z

There is a problem with throwing exceptions when the interface is consumed on a separate thread. (the reason why consumeOnAnotherThread is missing in the test case)

Quite like ok_http, if we throw the exception in that case, we run into a deadlock. For ok_http, it is default behaviour to terminate the connection with a java.net.SocketTimeoutException, but here, it pretty much leads to a forever wait cycle.

How could we solve this?

pkgs/jni/lib/src/jni.dart

pkgs/jnigen/pubspec.yaml

pkgs/jnigen/test/simple_package_test/runtime_test_registrant.dart

HosseinYousefi · 2024-06-20T12:47:22Z

There is a problem with throwing exceptions when the interface is consumed on a separate thread. (the reason why consumeOnAnotherThread is missing in the test case)

Thanks for letting me know, this is very important! I'll checkout your branch and debug why this happens.

coveralls · 2024-06-20T12:53:34Z

coverage: 92.069% (-0.05%) from 92.123%
when pulling d611416 on Anikate-De:support_java_excp
into a44ef95 on dart-lang:main.

This reverts commit 149b100.

Co-authored-by: Hossein Yousefi <[email protected]>

…t case Co-authored-by: Hossein Yousefi <[email protected]>

This reverts commit cbf184e.

HosseinYousefi · 2024-06-25T11:35:21Z

Added the test for the Future case.

Thanks @Anikate-De!

coveralls · 2024-06-25T11:41:30Z

coverage: 90.79% (-0.05%) from 90.841%
when pulling 3a7ff2a on Anikate-De:support_java_excp
into e5205d6 on dart-lang:main.

coveralls · 2024-06-25T11:41:49Z

coverage: 90.79% (-0.05%) from 90.841%
when pulling 3a7ff2a on Anikate-De:support_java_excp
into e5205d6 on dart-lang:main.

github-actions bot added package:jnigen package:jni labels Jun 17, 2024

HosseinYousefi reviewed Jun 18, 2024

View reviewed changes

pkgs/jni/src/dartjni.c Show resolved Hide resolved

pkgs/jnigen/lib/src/bindings/dart_generator.dart Outdated Show resolved Hide resolved

Anikate-De marked this pull request as ready for review June 19, 2024 06:30

Anikate-De requested a review from HosseinYousefi June 19, 2024 06:32

HosseinYousefi requested changes Jun 19, 2024

View reviewed changes

HosseinYousefi requested changes Jun 20, 2024

View reviewed changes

pkgs/jni/lib/src/jni.dart Outdated Show resolved Hide resolved

pkgs/jnigen/pubspec.yaml Outdated Show resolved Hide resolved

pkgs/jnigen/test/simple_package_test/runtime_test_registrant.dart Outdated Show resolved Hide resolved

HosseinYousefi mentioned this pull request Jun 20, 2024

Convert Java Futures to Dart Futures #1213

Open

Anikate-De and others added 13 commits June 21, 2024 15:50

added nullable property cause

1deb72d

generate FFI bindings

07b5030

add nullable cause property to jni extensions

8ea636e

check if thrown exceptions are of the type java.lang.Throwable

e7e6d49

[TEMP] edit kotlin_plugin example to demo exception handling

68206e5

Revert "[TEMP] edit kotlin_plugin example to demo exception handling"

83d4b9a

This reverts commit 149b100.

shift exception type checking to ProtectedJniExtensions

50ee57b

formatting and misc to pass ci/cd

2331993

added java exception test case

8c0f0d5

use google-java-format

8bab015

Co-authored-by: Hossein Yousefi <[email protected]>

create a static final field to denote jThrowableClass; simplify tes…

428f0a1

…t case Co-authored-by: Hossein Yousefi <[email protected]>

bump up version to 0.9.3-wip

169a1c1

format jni src files

c10e189

Address comments

132972f

HosseinYousefi force-pushed the support_java_excp branch from efbfaca to 132972f Compare June 21, 2024 13:53

Make jThrowableClass private

fe3966e

HosseinYousefi added the skip-changelog-check label Jun 21, 2024

HosseinYousefi added 6 commits June 21, 2024 16:12

Fix tests

e263766

Fix void type

cd6e720

[temp] debug test

cbf184e

Revert "[temp] debug test"

b5aee27

This reverts commit cbf184e.

Work around the tests with coverage

03f1162

Remove extra cast

3a7ff2a

HosseinYousefi self-requested a review June 25, 2024 11:31

HosseinYousefi approved these changes Jun 25, 2024

View reviewed changes

HosseinYousefi linked an issue Jun 25, 2024 that may be closed by this pull request

Support throwing Java exceptions in interface implementation #1209

Closed

HosseinYousefi merged commit bd5b751 into dart-lang:main Jun 25, 2024
26 checks passed

Anikate-De mentioned this pull request Jun 26, 2024

pkgs/ok_http: DevTools Networking Support. dart-lang/http#1242

Merged

6 tasks

pkgs/jni: Support Throwing Java Exceptions in Interface Impl. #1211

pkgs/jni: Support Throwing Java Exceptions in Interface Impl. #1211

Uh oh!

Conversation

Anikate-De commented Jun 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Anikate-De commented Jun 17, 2024

Uh oh!

Anikate-De commented Jun 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HosseinYousefi commented Jun 18, 2024

Uh oh!

github-actions bot commented Jun 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Health

Coverage ⚠️

License Headers ⚠️

Package publish validation ✔️

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HosseinYousefi commented Jun 19, 2024

Uh oh!

Anikate-De commented Jun 20, 2024

Uh oh!

Anikate-De commented Jun 20, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HosseinYousefi commented Jun 20, 2024

Uh oh!

coveralls commented Jun 20, 2024

Uh oh!

HosseinYousefi commented Jun 25, 2024

Uh oh!

Uh oh!

coveralls commented Jun 25, 2024

Uh oh!

coveralls commented Jun 25, 2024

Uh oh!

Uh oh!

Anikate-De commented Jun 17, 2024 •

edited

Loading

Anikate-De commented Jun 17, 2024 •

edited

Loading

github-actions bot commented Jun 19, 2024 •

edited

Loading