Summary: Add ExecutorchRuntimeException: Throw relevant exceptions from JNI layer in the event of errors #13526

psiddh · 2025-08-19T22:18:50Z

Add ExecutorchRuntimeException

Test Plan: Tested with executorch-examples and llama android demo.

Reviewers:

Subscribers:

Tasks:

Tags:

Summary

[PLEASE REMOVE] See CONTRIBUTING.md's Pull Requests for ExecuTorch PR guidelines.

[PLEASE REMOVE] If this PR closes an issue, please add a Fixes #<issue-id> line.

[PLEASE REMOVE] If this PR introduces a fix or feature that should be the upcoming release notes, please add a "Release notes: " label. For a list of available release notes labels, check out CONTRIBUTING.md's Pull Requests.

Test plan

[PLEASE REMOVE] How did you test this PR? Please write down any manual commands you used and note down tests that you have written if applicable.

pytorch-bot · 2025-08-19T22:18:53Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/13526

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❌ 3 New Failures

As of commit e9cf866 with merge base 6e520e2 ():

NEW FAILURES - The following jobs have failed:

Build documentation / build (buck2) / Build doc (gh)
At least one of the pre-conditions you specified did not hold
pull / unittest-arm-backend-with-no-fvp (test_pytest_models) / linux-job (gh)
RuntimeError: Command docker exec -t 74a95075d03e8be141796b56ae6577d4113c4c6909e19d0e9fed58e06454dec7 /exec failed with exit code 1
pull / unittest-arm-backend-with-no-fvp (test_pytest_ops) / linux-job (gh)
RuntimeError: Command docker exec -t c642d171bdda86ed874929160b06b3f93f509de9b90b39040a96e522048f028a /exec failed with exit code 1

This comment was automatically generated by Dr. CI and updates every 15 minutes.

github-actions · 2025-08-19T22:19:40Z

This PR needs a `release notes:` label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

JacobSzwejbka · 2025-08-20T20:49:31Z

extension/android/jni/BUCK

 non_fbcode_target(_kind = fb_android_cxx_library,
    name = "executorch_jni",
-    srcs = ["jni_layer.cpp", "log.cpp", "jni_layer_runtime.cpp"],
+    srcs = ["jni_layer.cpp", "log.cpp", "jni_layer_runtime.cpp", "jni_helper.cpp"],


can you refactor these srcs into a list that can be shared across the libs

Android Jni layer Test Plan: Tested executorch.jar with executorch demo example and llama demo app Reviewers: Subscribers: Tasks: Tags:

GregoryComer

Overall, looks great to me. Thanks for taking this. The only thing I wanted to get your thoughts on is whether we should update the Java definition in Module.java to indicate that it throws. I'm not as familiar with some of the Java conventions, but I believe we can add a checked exception specification to the Java declarations of the JNI methods to indicate that they can throw an ExecutorchRuntimeException.

From what I've seen, opinion seems to be divided on checked exceptions. I don't have a strong opinion either way, and it would potentially require some synchronous code changes to land internally. The advantage might be that it's more clear in the signature that various methods (Module.load / execute) can throw an ExecutorchRuntimeException.

GregoryComer · 2025-08-22T00:00:37Z

extension/android/jni/jni_layer.cpp

+
+      jni_helper::throwExecutorchException(
+          static_cast<uint32_t>(
+              Error::InvalidArgument), // For backward compatibility


From looking at user code, I'm a bit less worried about about strict BC here, so I think you could use the actual error code and not worry about matching the existing exception type.

Agree, it is always better to throw the exception with native error code as-is. I wasn't sure 100% abt it, but since you checked the client code, I will make the change

GregoryComer · 2025-08-22T00:07:51Z

You're probably already be planning this, but could you add/modify an integration test to cover thrown exceptions?

GregoryComer

Talked a bit with Sid offline. Sounds like unchecked exceptions are the better option here, which is good with me.

psiddh · 2025-08-22T01:26:05Z

From

If the native side (JNI C++ code) throws a RuntimeException or IllegalArgumentException (which is what we are doing now), these are unchecked exceptions in Java afaik.

Unchecked exceptions do not need to be declared in the Java method signature with throws.
It is considered good practice to omit throws for unchecked exceptions because:
These exceptions represent programming errors or unexpected conditions.Declaring them clutters the API without adding compile-time safety.

psiddh · 2025-08-22T01:27:07Z

You're probably already be planning this, but could you add/modify an integration test to cover thrown exceptions?

Yes was planning on a follow up change

…om JNI layer in the event of errors (pytorch#13526) Add ExecutorchRuntimeException Test Plan: Tested with executorch-examples and llama android demo. Reviewers: Subscribers: Tasks: Tags: ### Summary [PLEASE REMOVE] See [CONTRIBUTING.md's Pull Requests](https://github.com/pytorch/executorch/blob/main/CONTRIBUTING.md#pull-requests) for ExecuTorch PR guidelines. [PLEASE REMOVE] If this PR closes an issue, please add a `Fixes #<issue-id>` line. [PLEASE REMOVE] If this PR introduces a fix or feature that should be the upcoming release notes, please add a "Release notes: <area>" label. For a list of available release notes labels, check out [CONTRIBUTING.md's Pull Requests](https://github.com/pytorch/executorch/blob/main/CONTRIBUTING.md#pull-requests). ### Test plan [PLEASE REMOVE] How did you test this PR? Please write down any manual commands you used and note down tests that you have written if applicable. Co-authored-by: Github Executorch <github_executorch@arm.com>

…tions from JNI layer in the event of errors (#13526)" This reverts commit 8aca197.

Revert #13526 and #13640 to land as one internally

…om JNI layer in the event of errors (pytorch#13526) Summary: Add ExecutorchRuntimeException Reviewed By: jackzhxng Differential Revision: D81076817

…om JNI layer in the event of errors (#13526) Differential Revision: D81076817 Pull Request resolved: #13714

meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 19, 2025

psiddh force-pushed the android_exceptions branch 3 times, most recently from 4f0f4d2 to bc8f701 Compare August 20, 2025 16:32

psiddh requested review from GregoryComer and JacobSzwejbka August 20, 2025 19:20

psiddh changed the title ~~Summary: Add initial basic infra to throw relevant exceptions from~~ Summary: Add ExecutorchRuntimeException: Used to throw relevant exceptions from JNI layer in the event of errors Aug 20, 2025

psiddh changed the title ~~Summary: Add ExecutorchRuntimeException: Used to throw relevant exceptions from JNI layer in the event of errors~~ Summary: Add ExecutorchRuntimeException: Throw relevant exceptions from JNI layer in the event of errors Aug 20, 2025

JacobSzwejbka reviewed Aug 20, 2025

View reviewed changes

psiddh force-pushed the android_exceptions branch 2 times, most recently from f48dd0c to 5409ecd Compare August 21, 2025 03:57

Summary: Add initial basic infra to throw relevant exceptions from

e9cf866

Android Jni layer Test Plan: Tested executorch.jar with executorch demo example and llama demo app Reviewers: Subscribers: Tasks: Tags:

psiddh force-pushed the android_exceptions branch from 5409ecd to e9cf866 Compare August 21, 2025 04:15

psiddh marked this pull request as ready for review August 21, 2025 16:31

psiddh requested review from kirklandsign and larryliu0820 as code owners August 21, 2025 16:31

psiddh mentioned this pull request Aug 21, 2025

Android GA: Add Exceptions in JNI layer, Establish the foundational pieces #13583

Closed

GregoryComer reviewed Aug 22, 2025

View reviewed changes

GregoryComer approved these changes Aug 22, 2025

View reviewed changes

psiddh merged commit 8aca197 into main Aug 22, 2025
101 of 104 checks passed

psiddh deleted the android_exceptions branch August 22, 2025 06:18

jackzhxng added a commit that referenced this pull request Aug 26, 2025

Revert "Summary: Add ExecutorchRuntimeException: Throw relevant excep…

fb63bf3

…tions from JNI layer in the event of errors (#13526)" This reverts commit 8aca197.

jackzhxng mentioned this pull request Aug 26, 2025

Revert #13526 and #13640 #13684

Merged

jackzhxng added a commit that referenced this pull request Aug 26, 2025

Revert #13526 and #13640 (#13684)

dd16790

Revert #13526 and #13640 to land as one internally

facebook-github-bot pushed a commit that referenced this pull request Aug 27, 2025

Summary: Add ExecutorchRuntimeException: Throw relevant exceptions fr…

8278e7b

…om JNI layer in the event of errors (#13526) Differential Revision: D81076817 Pull Request resolved: #13714

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Summary: Add ExecutorchRuntimeException: Throw relevant exceptions from JNI layer in the event of errors #13526

Summary: Add ExecutorchRuntimeException: Throw relevant exceptions from JNI layer in the event of errors #13526

Uh oh!

psiddh commented Aug 19, 2025 •

edited

Loading

Uh oh!

pytorch-bot bot commented Aug 19, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 19, 2025

Uh oh!

JacobSzwejbka Aug 20, 2025

Uh oh!

psiddh Aug 21, 2025

Uh oh!

GregoryComer left a comment

Uh oh!

GregoryComer Aug 22, 2025

Uh oh!

psiddh Aug 22, 2025

Uh oh!

GregoryComer commented Aug 22, 2025

Uh oh!

GregoryComer left a comment

Uh oh!

psiddh commented Aug 22, 2025

Uh oh!

psiddh commented Aug 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Summary: Add ExecutorchRuntimeException: Throw relevant exceptions from JNI layer in the event of errors #13526

Summary: Add ExecutorchRuntimeException: Throw relevant exceptions from JNI layer in the event of errors #13526

Uh oh!

Conversation

psiddh commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

pytorch-bot bot commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/13526

❌ 3 New Failures

Uh oh!

github-actions bot commented Aug 19, 2025

This PR needs a release notes: label

Uh oh!

JacobSzwejbka Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

psiddh Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

GregoryComer left a comment

Choose a reason for hiding this comment

Uh oh!

GregoryComer Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

psiddh Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

GregoryComer commented Aug 22, 2025

Uh oh!

GregoryComer left a comment

Choose a reason for hiding this comment

Uh oh!

psiddh commented Aug 22, 2025

Uh oh!

psiddh commented Aug 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

psiddh commented Aug 19, 2025 •

edited

Loading

pytorch-bot bot commented Aug 19, 2025 •

edited

Loading

This PR needs a `release notes:` label