Android: unified error reporting — all sync errors throw, no silent f…

[psiddh]

…ailures

Replace the mixed error-reporting pattern (int return codes, empty arrays, silent no-ops) with a consistent exception-based contract across Module, LlmModule, and LlmCallback:

• Module: all public methods acquire mLock and check destroyed state; loadMethod() throws ExecutorchRuntimeException on native error; execute()/forward() throw IllegalStateException on destroyed module; destroy() throws if lock unavailable (concurrent execution)

• LlmModule: all generate/load/prefill methods check destroyed state via volatile flag and throw on native errors; resetNative() deprecated in favor of future close(); stop() intentionally unguarded for interrupt-during-generate; prefill methods return void instead of long

• LlmCallback: onError() default logs via android.util.Log with try/catch fallback for JVM unit test environments

• ExecutorchRuntimeException: added ALREADY_LOADED (0x04) error code, javadoc on all 19 error codes, "ExecuTorch" casing in error messages

• JNI: renamed registrations to match Java native method names (generateNative, loadNative, resetContextNative); removed double exception throw from C++ load(); unknown input typeCode now throws

• Tests: updated for void returns and assertThrows; all @ignore preserved

• Benchmark: ModelRunner and LlmModelRunner adapted to try/catch pattern

• All APIs are still experimental.

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• [CI[B200] Smoke test encounters CUDA Unknown error for dgxb200-03 and dgxb200-04

✅ You can merge normally! (3 Unrelated Failures)

As of commit 8af2e42 with merge base d93ef2c ():

FLAKY - The following job failed but was likely due to flakiness present on trunk:

• pull / test-moshi-linux / linux-job (gh) (matched linux rule in flaky-rules.json)
  E: Failed to fetch http://archive.ubuntu.com/ubuntu/pool/universe/p/pocketsphinx/pocketsphinx-en-us_0.8.0%2breal5prealpha%2b1-14ubuntu1_all.deb Unable to connect to archive.ubuntu.com:80: [IP: 91.189.91.81 80]

BROKEN TRUNK - The following jobs failed but was present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

• pull / unittest / windows / windows-job (gh) (trunk failure)
  ##[error]The operation was canceled.
• pull / unittest-editable / windows / windows-job (gh) (trunk failure)
  ##[error]The operation was canceled.

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

[github-actions]

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.

[Copilot]

Pull request overview

This PR standardizes Android-side error reporting by shifting ExecuTorch Java APIs from mixed return-code patterns to an exception-based contract, and aligning JNI registrations with the updated native method names.

Changes:

• Updated Module and LlmModule public APIs to throw on errors/destroyed-state instead of returning status codes or empty results.
• Renamed/adjusted JNI native method registrations to match updated Java native method names and added stricter input-type validation.
• Adapted Android benchmark and instrumentation tests to the new void/exception-based APIs.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
extension/benchmark/android/benchmark/app/src/main/java/org/pytorch/minibench/ModelRunner.java	Adapts benchmarking to exception-based loadMethod() while preserving a numeric status metric.
extension/benchmark/android/benchmark/app/src/main/java/org/pytorch/minibench/LlmModelRunner.java	Converts load() to try/catch error-code extraction and logs generation failures.
extension/android/jni/jni_layer.cpp	Throws on unsupported input EValue type codes; renames registered natives for etdump/getMethods.
extension/android/jni/jni_layer_llama.cpp	Simplifies load() to return error codes only; renames registered natives to *Native variants.
extension/android/executorch_android/src/main/java/org/pytorch/executorch/Module.java	Enforces locking + destroyed checks across public methods; converts loadMethod() to void that throws on native errors; wraps getMethods/etdump.
extension/android/executorch_android/src/main/java/org/pytorch/executorch/extension/llm/LlmModule.java	Adds destroyed-state guard; converts generate/load/prefill methods to throw exceptions and return void.
extension/android/executorch_android/src/main/java/org/pytorch/executorch/extension/llm/LlmCallback.java	Adds a default onError() implementation that logs to Logcat with a fallback path.
extension/android/executorch_android/src/main/java/org/pytorch/executorch/ExecutorchRuntimeException.java	Adds ALREADY_LOADED, expands error-code Javadoc, and standardizes “ExecuTorch” casing in formatted messages.
extension/android/executorch_android/src/androidTest/java/org/pytorch/executorch/ModuleInstrumentationTest.kt	Updates tests for exception-based loadMethod() and destroyed-module behavior.
extension/android/executorch_android/src/androidTest/java/org/pytorch/executorch/LlmModuleInstrumentationTest.kt	Updates LLM instrumentation test to the new void load() API.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (3)

extension/android/executorch_android/src/main/java/org/pytorch/executorch/extension/llm/LlmModule.java:276

• These native methods are registered by name via fbjni. Without @DoNotStrip (or an explicit consumer ProGuard/R8 keep rule), their names may be obfuscated, causing JNI registration to fail at runtime. Please annotate generateNative() (and other registered native methods) with @DoNotStrip or add appropriate keep rules for native members.

      int numEos) {
    checkNotDestroyed();
    int err = generateNative(prompt, seqLen, llmCallback, echo, temperature, numBos, numEos);
    if (err != 0) {
      throw ExecutorchRuntimeException.makeExecutorchException(err, "Failed to generate");
    }
  }

  private native int generateNative(
      String prompt,
      int seqLen,
      LlmCallback llmCallback,
      boolean echo,

extension/android/executorch_android/src/androidTest/java/org/pytorch/executorch/ModuleInstrumentationTest.kt:117

• After Module.destroy(), loadMethod() throws IllegalStateException. This assertion uses the broad RuntimeException type, which could hide regressions (e.g., wrong exception type). Prefer asserting IllegalStateException specifically here.

    module.loadMethod(FORWARD_METHOD)
  }

  @Test
  @Throws(IOException::class)
  fun testLoadOnDestroyedModule() {
    val module = Module.load(getTestFilePath(TEST_FILE_NAME))

extension/benchmark/android/benchmark/app/src/main/java/org/pytorch/minibench/ModelRunner.java:43

• If loadMethod("forward") throws, this code records an errorCode but then still runs warmup/benchmark forward() calls and etdump(). With the new exception-based contract, those calls can also throw and abort the benchmark, preventing metrics from being recorded. Consider returning early (or wrapping forward()/etdump() in try/catch and reporting an appropriate load_status) when preloading fails.

    Module module = Module.load(model.getPath());
    int errorCode = 0;
    try {
      module.loadMethod("forward");
    } catch (Exception e) {
      errorCode =
          (e instanceof org.pytorch.executorch.ExecutorchRuntimeException)
              ? ((org.pytorch.executorch.ExecutorchRuntimeException) e).getErrorCode()
              : -1;
    }
    long loadEnd = System.nanoTime();

    for (int i = 0; i < numWarmupIter; i++) {
      module.forward();
    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

extension/android/executorch_android/src/main/java/org/pytorch/executorch/extension/llm/LlmModule.java:697

• stop() is still a direct native method and does not consult mDestroyed / checkNotDestroyed(), unlike the other public APIs. After resetNative() sets mDestroyed = true and resets the underlying HybridData, a subsequent stop() call can invoke JNI on a destroyed hybrid object and fail unexpectedly. Consider making stop() a Java wrapper that checks destroyed state (and delegates to a renamed private native like stopNative()), or otherwise ensure stop() is safe/defined after destruction.

  /** Stop current generate() before it finishes. */
  @DoNotStrip
  public native void stop();

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kirklandsign]

Will it break existing users? If they expect an int return value

[psiddh]

Good catch! Yes this is an intentional breaking change (all apis are experimental, so we can still api signature) This is a good a long term solution for error capturing with try catch, as it forces Apps to add the try catch handler and handle errors gracefully.
The executorch-examples LlamaDemo app already uses this pattern and is unaffected. The only external breakage is SanityCheck.kt in executorch-examples, which needs a one-line fix, a follow up PR is planned after this lands.
Wdyt ?

[kirklandsign]

That works. We also need to check fb internal use cases

[meta-codesync]

@psiddh has imported this pull request. If you are a Meta employee, you can view this in D101260086.