Back out "Re-apply D101260086: Android unified error reporting"

Summary:
Original commit changeset: afc9ba215caa

Original Phabricator Diff: D102493794

Differential Revision: D102493838

Author: JacobSzwejbka

extension/android/executorch_android/src/androidTest/java/org/pytorch/executorch/LlmModuleInstrumentationTest.kt

@@ -54,7 +54,9 @@ class LlmModuleInstrumentationTest : LlmCallback {
   @Test
   @Throws(IOException::class, URISyntaxException::class)
   fun testGenerate() {
-    llmModule.load()
+    val loadResult = llmModule.load()
+    // Check that the model can be load successfully
+    assertEquals(OK.toLong(), loadResult.toLong())
 
     llmModule.generate(TEST_PROMPT, SEQ_LEN, this@LlmModuleInstrumentationTest)
     assertEquals(results.size.toLong(), SEQ_LEN.toLong())
@@ -275,6 +277,7 @@ class LlmModuleInstrumentationTest : LlmCallback {
     private const val TEST_FILE_NAME = "/stories.pte"
     private const val TOKENIZER_FILE_NAME = "/tokenizer.bin"
     private const val TEST_PROMPT = "Hello"
+    private const val OK = 0x00
     private const val SEQ_LEN = 32
   }
 }

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

@@ -66,7 +66,8 @@ class ModuleInstrumentationTest {
   fun testModuleLoadMethodAndForward() {
     val module = Module.load(getTestFilePath(TEST_FILE_NAME))
 
-    module.loadMethod(FORWARD_METHOD)
+    val loadMethod = module.loadMethod(FORWARD_METHOD)
+    Assert.assertEquals(loadMethod.toLong(), OK.toLong())
 
     val results = module.forward()
     Assert.assertTrue(results[0].isTensor)
@@ -95,22 +96,17 @@ class ModuleInstrumentationTest {
   fun testModuleLoadMethodNonExistantMethod() {
     val module = Module.load(getTestFilePath(TEST_FILE_NAME))
 
-    val exception =
-        Assert.assertThrows(ExecutorchRuntimeException::class.java) {
-          module.loadMethod(NONE_METHOD)
-        }
-    Assert.assertEquals(
-        ExecutorchRuntimeException.INVALID_ARGUMENT,
-        exception.getErrorCode(),
-    )
+    val loadMethod = module.loadMethod(NONE_METHOD)
+    Assert.assertEquals(loadMethod.toLong(), INVALID_ARGUMENT.toLong())
   }
 
   @Test(expected = RuntimeException::class)
   @Throws(IOException::class)
   fun testNonPteFile() {
     val module = Module.load(getTestFilePath(NON_PTE_FILE_NAME))
 
-    module.loadMethod(FORWARD_METHOD)
+    val loadMethod = module.loadMethod(FORWARD_METHOD)
+    Assert.assertEquals(loadMethod.toLong(), INVALID_ARGUMENT.toLong())
   }
 
   @Test
@@ -120,19 +116,22 @@ class ModuleInstrumentationTest {
 
     module.destroy()
 
-    Assert.assertThrows(IllegalStateException::class.java) { module.loadMethod(FORWARD_METHOD) }
+    val loadMethod = module.loadMethod(FORWARD_METHOD)
+    Assert.assertEquals(loadMethod.toLong(), INVALID_STATE.toLong())
   }
 
   @Test
   @Throws(IOException::class)
   fun testForwardOnDestroyedModule() {
     val module = Module.load(getTestFilePath(TEST_FILE_NAME))
 
-    module.loadMethod(FORWARD_METHOD)
+    val loadMethod = module.loadMethod(FORWARD_METHOD)
+    Assert.assertEquals(loadMethod.toLong(), OK.toLong())
 
     module.destroy()
 
-    Assert.assertThrows(IllegalStateException::class.java) { module.forward() }
+    val results = module.forward()
+    Assert.assertEquals(0, results.size.toLong())
   }
 
   @Ignore(
@@ -176,5 +175,9 @@ class ModuleInstrumentationTest {
     private const val NON_PTE_FILE_NAME = "/test.txt"
     private const val FORWARD_METHOD = "forward"
     private const val NONE_METHOD = "none"
+    private const val OK = 0x00
+    private const val INVALID_STATE = 0x2
+    private const val INVALID_ARGUMENT = 0x12
+    private const val ACCESS_FAILED = 0x22
   }
 }

extension/android/executorch_android/src/main/java/org/pytorch/executorch/ExecutorchRuntimeException.java

@@ -12,83 +12,34 @@
 import java.util.HashMap;
 import java.util.Map;
 
-/**
- * Base exception for all ExecuTorch runtime errors. Each instance carries an integer error code
- * corresponding to the native {@code runtime/core/error.h} values, accessible via {@link
- * #getErrorCode()}.
- */
 public class ExecutorchRuntimeException extends RuntimeException {
   // Error code constants - keep in sync with runtime/core/error.h
-
   // System errors
-
-  /** Operation completed successfully. */
   public static final int OK = 0x00;
-
-  /** An unexpected internal error occurred in the runtime. */
   public static final int INTERNAL = 0x01;
-
-  /** The runtime or method is in an invalid state for the requested operation. */
   public static final int INVALID_STATE = 0x02;
-
-  /** The method has finished execution and has no more work to do. */
   public static final int END_OF_METHOD = 0x03;
 
-  /** A required resource has already been loaded. */
-  public static final int ALREADY_LOADED = 0x04;
-
   // Logical errors
-
-  /** The requested operation is not supported by this build or backend. */
   public static final int NOT_SUPPORTED = 0x10;
-
-  /** The requested operation has not been implemented. */
   public static final int NOT_IMPLEMENTED = 0x11;
-
-  /** One or more arguments passed to the operation are invalid. */
   public static final int INVALID_ARGUMENT = 0x12;
-
-  /** A value or tensor has an unexpected type. */
   public static final int INVALID_TYPE = 0x13;
-
-  /** A required operator kernel is not registered. */
   public static final int OPERATOR_MISSING = 0x14;
-
-  /** The maximum number of registered kernels has been exceeded. */
   public static final int REGISTRATION_EXCEEDING_MAX_KERNELS = 0x15;
-
-  /** A kernel with the same name is already registered. */
   public static final int REGISTRATION_ALREADY_REGISTERED = 0x16;
 
   // Resource errors
-
-  /** A required resource (file, tensor, program) was not found. */
   public static final int NOT_FOUND = 0x20;
-
-  /** A memory allocation failed. */
   public static final int MEMORY_ALLOCATION_FAILED = 0x21;
-
-  /** Access to a resource was denied or failed. */
   public static final int ACCESS_FAILED = 0x22;
-
-  /** The loaded program is malformed or incompatible. */
   public static final int INVALID_PROGRAM = 0x23;
-
-  /** External data referenced by the program is invalid or missing. */
   public static final int INVALID_EXTERNAL_DATA = 0x24;
-
-  /** The system has run out of a required resource. */
   public static final int OUT_OF_RESOURCES = 0x25;
 
   // Delegate errors
-
-  /** A delegate reported an incompatible model or configuration. */
   public static final int DELEGATE_INVALID_COMPATIBILITY = 0x30;
-
-  /** A delegate failed to allocate required memory. */
   public static final int DELEGATE_MEMORY_ALLOCATION_FAILED = 0x31;
-
-  /** A delegate received an invalid or stale handle. */
   public static final int DELEGATE_INVALID_HANDLE = 0x32;
 
   private static final Map<Integer, String> ERROR_CODE_MESSAGES;
@@ -101,7 +52,6 @@ public class ExecutorchRuntimeException extends RuntimeException {
     map.put(INTERNAL, "Internal error");
     map.put(INVALID_STATE, "Invalid state");
     map.put(END_OF_METHOD, "End of method reached");
-    map.put(ALREADY_LOADED, "Already loaded");
     // Logical errors
     map.put(NOT_SUPPORTED, "Operation not supported");
     map.put(NOT_IMPLEMENTED, "Operation not implemented");
@@ -133,7 +83,7 @@ static String formatMessage(int errorCode, String details) {
 
       String safeDetails = details != null ? details : "No details provided";
       return String.format(
-          "[ExecuTorch Error 0x%s] %s: %s",
+          "[Executorch Error 0x%s] %s: %s",
           Integer.toHexString(errorCode), baseMessage, safeDetails);
     }
 
@@ -161,12 +111,10 @@ public ExecutorchRuntimeException(int errorCode, String details) {
     this.errorCode = errorCode;
   }
 
-  /** Returns the numeric error code from {@code runtime/core/error.h}. */
   public int getErrorCode() {
     return errorCode;
   }
 
-  /** Returns detailed log output captured from the native runtime, if available. */
   public String getDetailedError() {
     return ErrorHelper.getDetailedErrorLogs();
   }

extension/android/executorch_android/src/main/java/org/pytorch/executorch/Module.java

@@ -8,6 +8,7 @@
 
 package org.pytorch.executorch;
 
+import android.util.Log;
 import com.facebook.jni.HybridData;
 import com.facebook.jni.annotations.DoNotStrip;
 import com.facebook.soloader.nativeloader.NativeLoader;
@@ -129,10 +130,11 @@ public EValue[] forward(EValue... inputs) {
    * @return return value from the method.
    */
   public EValue[] execute(String methodName, EValue... inputs) {
-    mLock.lock();
     try {
+      mLock.lock();
       if (!mHybridData.isValid()) {
-        throw new IllegalStateException("Module has been destroyed");
+        Log.e("ExecuTorch", "Attempt to use a destroyed module");
+        return new EValue[0];
       }
       return executeNative(methodName, inputs);
     } finally {
@@ -149,17 +151,17 @@ public EValue[] execute(String methodName, EValue... inputs) {
    * synchronous, and will block until the method is loaded. Therefore, it is recommended to call
    * this on a background thread. However, users need to make sure that they don't execute before
    * this function returns.
+   *
+   * @return the Error code if there was an error loading the method
    */
-  public void loadMethod(String methodName) {
-    mLock.lock();
+  public int loadMethod(String methodName) {
     try {
+      mLock.lock();
       if (!mHybridData.isValid()) {
-        throw new IllegalStateException("Module has been destroyed");
-      }
-      int errorCode = loadMethodNative(methodName);
-      if (errorCode != 0) {
-        throw new ExecutorchRuntimeException(errorCode, "Failed to load method: " + methodName);
+        Log.e("ExecuTorch", "Attempt to use a destroyed module");
+        return 0x2; // InvalidState
       }
+      return loadMethodNative(methodName);
     } finally {
       mLock.unlock();
     }
@@ -182,20 +184,8 @@ public void loadMethod(String methodName) {
    *
    * @return name of methods in this Module
    */
-  public String[] getMethods() {
-    mLock.lock();
-    try {
-      if (!mHybridData.isValid()) {
-        throw new IllegalStateException("Module has been destroyed");
-      }
-      return getMethodsNative();
-    } finally {
-      mLock.unlock();
-    }
-  }
-
   @DoNotStrip
-  private native String[] getMethodsNative();
+  public native String[] getMethods();
 
   /**
    * Get the corresponding @MethodMetadata for a method
@@ -204,19 +194,11 @@ public String[] getMethods() {
    * @return @MethodMetadata for this method
    */
   public MethodMetadata getMethodMetadata(String name) {
-    mLock.lock();
-    try {
-      if (!mHybridData.isValid()) {
-        throw new IllegalStateException("Module has been destroyed");
-      }
-      MethodMetadata methodMetadata = mMethodMetadata.get(name);
-      if (methodMetadata == null) {
-        throw new IllegalArgumentException("method " + name + " does not exist for this module");
-      }
-      return methodMetadata;
-    } finally {
-      mLock.unlock();
+    MethodMetadata methodMetadata = mMethodMetadata.get(name);
+    if (methodMetadata == null) {
+      throw new IllegalArgumentException("method " + name + " does not exist for this module");
     }
+    return methodMetadata;
   }
 
   @DoNotStrip
@@ -228,15 +210,7 @@ public static String[] readLogBufferStatic() {
 
   /** Retrieve the in-memory log buffer, containing the most recent ExecuTorch log entries. */
   public String[] readLogBuffer() {
-    mLock.lock();
-    try {
-      if (!mHybridData.isValid()) {
-        throw new IllegalStateException("Module has been destroyed");
-      }
-      return readLogBufferNative();
-    } finally {
-      mLock.unlock();
-    }
+    return readLogBufferNative();
   }
 
   @DoNotStrip
@@ -250,20 +224,8 @@ public String[] readLogBuffer() {
    * @return true if the etdump was successfully written, false otherwise.
    */
   @Experimental
-  public boolean etdump() {
-    mLock.lock();
-    try {
-      if (!mHybridData.isValid()) {
-        throw new IllegalStateException("Module has been destroyed");
-      }
-      return etdumpNative();
-    } finally {
-      mLock.unlock();
-    }
-  }
-
   @DoNotStrip
-  private native boolean etdumpNative();
+  public native boolean etdump();
 
   /**
    * Explicitly destroys the native Module object. Calling this method is not required, as the
@@ -279,7 +241,10 @@ public void destroy() {
         mLock.unlock();
       }
     } else {
-      throw new IllegalStateException("Cannot destroy module while method is executing");
+      Log.w(
+          "ExecuTorch",
+          "Destroy was called while the module was in use. Resources will not be immediately"
+              + " released.");
     }
   }
 }

extension/android/executorch_android/src/main/java/org/pytorch/executorch/extension/llm/LlmCallback.java

@@ -46,11 +46,5 @@ default void onStats(String stats) {}
    * @param message Human-readable error description
    */
   @DoNotStrip
-  default void onError(int errorCode, String message) {
-    try {
-      android.util.Log.e("ExecuTorch", "LLM error " + errorCode + ": " + message);
-    } catch (Throwable t) {
-      System.err.println("ExecuTorch LLM error " + errorCode + ": " + message);
-    }
-  }
+  default void onError(int errorCode, String message) {}
 }