fix(JsBridge): meaningful error when module/method routing fails (#25)#40
fix(JsBridge): meaningful error when module/method routing fails (#25)#40jim-daf wants to merge 1 commit intopengwei1024:masterfrom
Conversation
…engwei1024#25) The error path in onCallJsPrompt reported "JBArgument Parse error" for two unrelated failure modes: module not registered and method not exposed. Developers reading the log assumed their JSON was malformed and went looking in the wrong place. This change splits the message into "Module not found" and "Method not found" so the actual cause is visible, and trims the incoming jsonString in JBArgumentParser so a stray whitespace from the prompt channel does not look like a malformed payload.
There was a problem hiding this comment.
Pull request overview
Improves JsBridge error reporting so JS callers get actionable messages when native module/method routing fails, and makes argument parsing more tolerant of leading whitespace.
Changes:
- Return specific
"Module not found: ..."/"Method not found: ..."errors instead of the misleading generic parse error. - Trim incoming JSON before validating/parsing to avoid failures due to leading whitespace.
- Adjust logging to align with the new failure modes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| jsbridge/src/main/java/com/apkfuns/jsbridge/JsBridgeImpl.java | Splits routing failure paths to report missing module/method explicitly. |
| jsbridge/src/main/java/com/apkfuns/jsbridge/JBArgumentParser.java | Trims JSON input before parsing to improve robustness against leading whitespace. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (true) { | ||
| JsMethod method = methodHashMap.get(argumentParser.getMethod()); | ||
| List<JBArgumentParser.Parameter> parameters = argumentParser.getParameters(); |
There was a problem hiding this comment.
The if (true) block is unconditional and appears to be leftover scaffolding from the previous nested if (findModule != null) logic. It adds an unnecessary indentation level and makes the control flow harder to follow. Remove the if (true) and unindent the enclosed code.
| // Issue #25: trim before the startsWith check so a leading | ||
| // whitespace or BOM coming through the prompt channel does | ||
| // not look like a malformed payload. | ||
| String trimmed = jsonString == null ? null : jsonString.trim(); |
There was a problem hiding this comment.
The comment says trimming handles a leading BOM, but String.trim() does not remove the UTF-8 BOM character (\uFEFF). Either strip a BOM explicitly (and document it), or remove the BOM claim from the comment to avoid misleading future maintainers.
| // Issue #25: trim before the startsWith check so a leading | |
| // whitespace or BOM coming through the prompt channel does | |
| // not look like a malformed payload. | |
| String trimmed = jsonString == null ? null : jsonString.trim(); | |
| // Issue #25: strip a leading BOM explicitly, then trim before | |
| // the startsWith check so leading whitespace or a BOM coming | |
| // through the prompt channel does not look like a malformed payload. | |
| String trimmed = jsonString; | |
| if (trimmed != null && trimmed.startsWith("\uFEFF")) { | |
| trimmed = trimmed.substring(1); | |
| } | |
| trimmed = trimmed == null ? null : trimmed.trim(); |
| if(TextUtils.isEmpty(trimmed) || (!trimmed.startsWith("{") && | ||
| !trimmed.startsWith("["))) { | ||
| parser.setThrowable(new JSONException("Argument error: " + jsonString)); | ||
| return parser; | ||
| } | ||
| try { | ||
| JSONObject jsonObject = new JSONObject(jsonString); | ||
| JSONObject jsonObject = new JSONObject(trimmed); |
There was a problem hiding this comment.
The startsWith("[") check suggests JSON arrays are supported, but the code always parses with new JSONObject(...), which will throw for array payloads. Either remove the [ acceptance here, or add proper JSONArray handling so the pre-check matches what the parser can actually consume.
Meaningful error when module or method routing fails (refs #25)
Resolves #25
Why "JBArgument Parse error" is misleading
In
JsBridgeImpl.onCallJsPromptthe error path is:The JSON itself parsed fine (
isSuccess()is true). The error fires when the named module or method is not registered on the Java side. Reporting that as "JBArgument Parse error" sends the developer looking at their JS payload, not at their Java module list. Issue #25 is exactly this misdirection.The fix
Split the failure modes:
The fail callback on the JS side now receives
"Module not found: networkInfo"or"Method not found: <module>.networkInfo"instead of the generic string, which is enough to debug the original report (the user was seeing intermittent calls fail under exactly this code path).Parser robustness
Also trim the incoming jsonString before the
startsWith("{")check so a stray leading whitespace or BOM coming throughonJsPromptdoes not look like a malformed payload:Files changed
jsbridge/src/main/java/com/apkfuns/jsbridge/JBArgumentParser.javajsbridge/src/main/java/com/apkfuns/jsbridge/JsBridgeImpl.java