Skip to content

Commit 65e793b

Browse files
psiddhclaude
andcommitted
Android: unified error reporting — all sync errors throw, no silent failures
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 Breaking change under @experimental — all APIs are still experimental. Co-authored-by: Claude <noreply@anthropic.com>
1 parent 60d57e5 commit 65e793b

File tree

10 files changed

+252
-120
lines changed

10 files changed

+252
-120
lines changed

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,7 @@ class LlmModuleInstrumentationTest : LlmCallback {
5454
@Test
5555
@Throws(IOException::class, URISyntaxException::class)
5656
fun testGenerate() {
57-
val loadResult = llmModule.load()
58-
// Check that the model can be load successfully
59-
assertEquals(OK.toLong(), loadResult.toLong())
57+
llmModule.load()
6058

6159
llmModule.generate(TEST_PROMPT, SEQ_LEN, this@LlmModuleInstrumentationTest)
6260
assertEquals(results.size.toLong(), SEQ_LEN.toLong())
@@ -277,7 +275,6 @@ class LlmModuleInstrumentationTest : LlmCallback {
277275
private const val TEST_FILE_NAME = "/stories.pte"
278276
private const val TOKENIZER_FILE_NAME = "/tokenizer.bin"
279277
private const val TEST_PROMPT = "Hello"
280-
private const val OK = 0x00
281278
private const val SEQ_LEN = 32
282279
}
283280
}

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

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ class ModuleInstrumentationTest {
6666
fun testModuleLoadMethodAndForward() {
6767
val module = Module.load(getTestFilePath(TEST_FILE_NAME))
6868

69-
val loadMethod = module.loadMethod(FORWARD_METHOD)
70-
Assert.assertEquals(loadMethod.toLong(), OK.toLong())
69+
module.loadMethod(FORWARD_METHOD)
7170

7271
val results = module.forward()
7372
Assert.assertTrue(results[0].isTensor)
@@ -96,17 +95,15 @@ class ModuleInstrumentationTest {
9695
fun testModuleLoadMethodNonExistantMethod() {
9796
val module = Module.load(getTestFilePath(TEST_FILE_NAME))
9897

99-
val loadMethod = module.loadMethod(NONE_METHOD)
100-
Assert.assertEquals(loadMethod.toLong(), INVALID_ARGUMENT.toLong())
98+
Assert.assertThrows(RuntimeException::class.java) { module.loadMethod(NONE_METHOD) }
10199
}
102100

103101
@Test(expected = RuntimeException::class)
104102
@Throws(IOException::class)
105103
fun testNonPteFile() {
106104
val module = Module.load(getTestFilePath(NON_PTE_FILE_NAME))
107105

108-
val loadMethod = module.loadMethod(FORWARD_METHOD)
109-
Assert.assertEquals(loadMethod.toLong(), INVALID_ARGUMENT.toLong())
106+
module.loadMethod(FORWARD_METHOD)
110107
}
111108

112109
@Test
@@ -116,22 +113,19 @@ class ModuleInstrumentationTest {
116113

117114
module.destroy()
118115

119-
val loadMethod = module.loadMethod(FORWARD_METHOD)
120-
Assert.assertEquals(loadMethod.toLong(), INVALID_STATE.toLong())
116+
Assert.assertThrows(RuntimeException::class.java) { module.loadMethod(FORWARD_METHOD) }
121117
}
122118

123119
@Test
124120
@Throws(IOException::class)
125121
fun testForwardOnDestroyedModule() {
126122
val module = Module.load(getTestFilePath(TEST_FILE_NAME))
127123

128-
val loadMethod = module.loadMethod(FORWARD_METHOD)
129-
Assert.assertEquals(loadMethod.toLong(), OK.toLong())
124+
module.loadMethod(FORWARD_METHOD)
130125

131126
module.destroy()
132127

133-
val results = module.forward()
134-
Assert.assertEquals(0, results.size.toLong())
128+
Assert.assertThrows(RuntimeException::class.java) { module.forward() }
135129
}
136130

137131
@Ignore(
@@ -175,9 +169,5 @@ class ModuleInstrumentationTest {
175169
private const val NON_PTE_FILE_NAME = "/test.txt"
176170
private const val FORWARD_METHOD = "forward"
177171
private const val NONE_METHOD = "none"
178-
private const val OK = 0x00
179-
private const val INVALID_STATE = 0x2
180-
private const val INVALID_ARGUMENT = 0x12
181-
private const val ACCESS_FAILED = 0x22
182172
}
183173
}

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

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,34 +12,83 @@
1212
import java.util.HashMap;
1313
import java.util.Map;
1414

15+
/**
16+
* Base exception for all ExecuTorch runtime errors. Each instance carries an integer error code
17+
* corresponding to the native {@code runtime/core/error.h} values, accessible via {@link
18+
* #getErrorCode()}.
19+
*/
1520
public class ExecutorchRuntimeException extends RuntimeException {
1621
// Error code constants - keep in sync with runtime/core/error.h
22+
1723
// System errors
24+
25+
/** Operation completed successfully. */
1826
public static final int OK = 0x00;
27+
28+
/** An unexpected internal error occurred in the runtime. */
1929
public static final int INTERNAL = 0x01;
30+
31+
/** The runtime or method is in an invalid state for the requested operation. */
2032
public static final int INVALID_STATE = 0x02;
33+
34+
/** The method has finished execution and has no more work to do. */
2135
public static final int END_OF_METHOD = 0x03;
2236

37+
/** A required resource has already been loaded. */
38+
public static final int ALREADY_LOADED = 0x04;
39+
2340
// Logical errors
41+
42+
/** The requested operation is not supported by this build or backend. */
2443
public static final int NOT_SUPPORTED = 0x10;
44+
45+
/** The requested operation has not been implemented. */
2546
public static final int NOT_IMPLEMENTED = 0x11;
47+
48+
/** One or more arguments passed to the operation are invalid. */
2649
public static final int INVALID_ARGUMENT = 0x12;
50+
51+
/** A value or tensor has an unexpected type. */
2752
public static final int INVALID_TYPE = 0x13;
53+
54+
/** A required operator kernel is not registered. */
2855
public static final int OPERATOR_MISSING = 0x14;
56+
57+
/** The maximum number of registered kernels has been exceeded. */
2958
public static final int REGISTRATION_EXCEEDING_MAX_KERNELS = 0x15;
59+
60+
/** A kernel with the same name is already registered. */
3061
public static final int REGISTRATION_ALREADY_REGISTERED = 0x16;
3162

3263
// Resource errors
64+
65+
/** A required resource (file, tensor, program) was not found. */
3366
public static final int NOT_FOUND = 0x20;
67+
68+
/** A memory allocation failed. */
3469
public static final int MEMORY_ALLOCATION_FAILED = 0x21;
70+
71+
/** Access to a resource was denied or failed. */
3572
public static final int ACCESS_FAILED = 0x22;
73+
74+
/** The loaded program is malformed or incompatible. */
3675
public static final int INVALID_PROGRAM = 0x23;
76+
77+
/** External data referenced by the program is invalid or missing. */
3778
public static final int INVALID_EXTERNAL_DATA = 0x24;
79+
80+
/** The system has run out of a required resource. */
3881
public static final int OUT_OF_RESOURCES = 0x25;
3982

4083
// Delegate errors
84+
85+
/** A delegate reported an incompatible model or configuration. */
4186
public static final int DELEGATE_INVALID_COMPATIBILITY = 0x30;
87+
88+
/** A delegate failed to allocate required memory. */
4289
public static final int DELEGATE_MEMORY_ALLOCATION_FAILED = 0x31;
90+
91+
/** A delegate received an invalid or stale handle. */
4392
public static final int DELEGATE_INVALID_HANDLE = 0x32;
4493

4594
private static final Map<Integer, String> ERROR_CODE_MESSAGES;
@@ -52,6 +101,7 @@ public class ExecutorchRuntimeException extends RuntimeException {
52101
map.put(INTERNAL, "Internal error");
53102
map.put(INVALID_STATE, "Invalid state");
54103
map.put(END_OF_METHOD, "End of method reached");
104+
map.put(ALREADY_LOADED, "Already loaded");
55105
// Logical errors
56106
map.put(NOT_SUPPORTED, "Operation not supported");
57107
map.put(NOT_IMPLEMENTED, "Operation not implemented");
@@ -83,7 +133,7 @@ static String formatMessage(int errorCode, String details) {
83133

84134
String safeDetails = details != null ? details : "No details provided";
85135
return String.format(
86-
"[Executorch Error 0x%s] %s: %s",
136+
"[ExecuTorch Error 0x%s] %s: %s",
87137
Integer.toHexString(errorCode), baseMessage, safeDetails);
88138
}
89139

@@ -111,10 +161,12 @@ public ExecutorchRuntimeException(int errorCode, String details) {
111161
this.errorCode = errorCode;
112162
}
113163

164+
/** Returns the numeric error code from {@code runtime/core/error.h}. */
114165
public int getErrorCode() {
115166
return errorCode;
116167
}
117168

169+
/** Returns detailed log output captured from the native runtime, if available. */
118170
public String getDetailedError() {
119171
return ErrorHelper.getDetailedErrorLogs();
120172
}

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

Lines changed: 57 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
package org.pytorch.executorch;
1010

11-
import android.util.Log;
1211
import com.facebook.jni.HybridData;
1312
import com.facebook.jni.annotations.DoNotStrip;
1413
import com.facebook.soloader.nativeloader.NativeLoader;
@@ -130,11 +129,10 @@ public EValue[] forward(EValue... inputs) {
130129
* @return return value from the method.
131130
*/
132131
public EValue[] execute(String methodName, EValue... inputs) {
132+
mLock.lock();
133133
try {
134-
mLock.lock();
135134
if (!mHybridData.isValid()) {
136-
Log.e("ExecuTorch", "Attempt to use a destroyed module");
137-
return new EValue[0];
135+
throw new IllegalStateException("Module has been destroyed");
138136
}
139137
return executeNative(methodName, inputs);
140138
} finally {
@@ -151,17 +149,17 @@ public EValue[] execute(String methodName, EValue... inputs) {
151149
* synchronous, and will block until the method is loaded. Therefore, it is recommended to call
152150
* this on a background thread. However, users need to make sure that they don't execute before
153151
* this function returns.
154-
*
155-
* @return the Error code if there was an error loading the method
156152
*/
157-
public int loadMethod(String methodName) {
153+
public void loadMethod(String methodName) {
154+
mLock.lock();
158155
try {
159-
mLock.lock();
160156
if (!mHybridData.isValid()) {
161-
Log.e("ExecuTorch", "Attempt to use a destroyed module");
162-
return 0x2; // InvalidState
157+
throw new IllegalStateException("Module has been destroyed");
158+
}
159+
int errorCode = loadMethodNative(methodName);
160+
if (errorCode != 0) {
161+
throw new ExecutorchRuntimeException(errorCode, "Failed to load method: " + methodName);
163162
}
164-
return loadMethodNative(methodName);
165163
} finally {
166164
mLock.unlock();
167165
}
@@ -184,8 +182,20 @@ public int loadMethod(String methodName) {
184182
*
185183
* @return name of methods in this Module
186184
*/
185+
public String[] getMethods() {
186+
mLock.lock();
187+
try {
188+
if (!mHybridData.isValid()) {
189+
throw new IllegalStateException("Module has been destroyed");
190+
}
191+
return getMethodsNative();
192+
} finally {
193+
mLock.unlock();
194+
}
195+
}
196+
187197
@DoNotStrip
188-
public native String[] getMethods();
198+
private native String[] getMethodsNative();
189199

190200
/**
191201
* Get the corresponding @MethodMetadata for a method
@@ -194,11 +204,19 @@ public int loadMethod(String methodName) {
194204
* @return @MethodMetadata for this method
195205
*/
196206
public MethodMetadata getMethodMetadata(String name) {
197-
MethodMetadata methodMetadata = mMethodMetadata.get(name);
198-
if (methodMetadata == null) {
199-
throw new IllegalArgumentException("method " + name + " does not exist for this module");
207+
mLock.lock();
208+
try {
209+
if (!mHybridData.isValid()) {
210+
throw new IllegalStateException("Module has been destroyed");
211+
}
212+
MethodMetadata methodMetadata = mMethodMetadata.get(name);
213+
if (methodMetadata == null) {
214+
throw new IllegalArgumentException("method " + name + " does not exist for this module");
215+
}
216+
return methodMetadata;
217+
} finally {
218+
mLock.unlock();
200219
}
201-
return methodMetadata;
202220
}
203221

204222
@DoNotStrip
@@ -210,7 +228,15 @@ public static String[] readLogBufferStatic() {
210228

211229
/** Retrieve the in-memory log buffer, containing the most recent ExecuTorch log entries. */
212230
public String[] readLogBuffer() {
213-
return readLogBufferNative();
231+
mLock.lock();
232+
try {
233+
if (!mHybridData.isValid()) {
234+
throw new IllegalStateException("Module has been destroyed");
235+
}
236+
return readLogBufferNative();
237+
} finally {
238+
mLock.unlock();
239+
}
214240
}
215241

216242
@DoNotStrip
@@ -224,8 +250,20 @@ public String[] readLogBuffer() {
224250
* @return true if the etdump was successfully written, false otherwise.
225251
*/
226252
@Experimental
253+
public boolean etdump() {
254+
mLock.lock();
255+
try {
256+
if (!mHybridData.isValid()) {
257+
throw new IllegalStateException("Module has been destroyed");
258+
}
259+
return etdumpNative();
260+
} finally {
261+
mLock.unlock();
262+
}
263+
}
264+
227265
@DoNotStrip
228-
public native boolean etdump();
266+
private native boolean etdumpNative();
229267

230268
/**
231269
* Explicitly destroys the native Module object. Calling this method is not required, as the
@@ -241,10 +279,7 @@ public void destroy() {
241279
mLock.unlock();
242280
}
243281
} else {
244-
Log.w(
245-
"ExecuTorch",
246-
"Destroy was called while the module was in use. Resources will not be immediately"
247-
+ " released.");
282+
throw new IllegalStateException("Cannot destroy module while method is executing");
248283
}
249284
}
250285
}

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,11 @@ default void onStats(String stats) {}
4646
* @param message Human-readable error description
4747
*/
4848
@DoNotStrip
49-
default void onError(int errorCode, String message) {}
49+
default void onError(int errorCode, String message) {
50+
try {
51+
android.util.Log.e("ExecuTorch", "LLM error " + errorCode + ": " + message);
52+
} catch (RuntimeException e) {
53+
System.err.println("ExecuTorch LLM error " + errorCode + ": " + message);
54+
}
55+
}
5056
}

0 commit comments

Comments
 (0)