Skip to content

fix(JsBridge): meaningful error when module/method routing fails (#25) #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jim-daf wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(JsBridge): meaningful error when module/method routing fails (#25) #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions jsbridge/src/main/java/com/apkfuns/jsbridge/JBArgumentParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,17 @@ public void setValue(String value) {
@NonNull
static JBArgumentParser parse(@Nullable String jsonString) {
JBArgumentParser parser = new JBArgumentParser();
if(TextUtils.isEmpty(jsonString) || (!jsonString.startsWith("{") &&
!jsonString.startsWith("["))) {
// 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();
Comment on lines +121 to +124
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
// 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();

Copilot uses AI. Check for mistakes.
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);
Comment on lines +125 to +131
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
parser.setId(jsonObject.optLong("id"));
parser.setMethod(jsonObject.optString("method"));
parser.setModule(jsonObject.optString("module"));
Expand Down
26 changes: 19 additions & 7 deletions jsbridge/src/main/java/com/apkfuns/jsbridge/JsBridgeImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,25 @@ private boolean onCallJsPrompt(String methodArgs, Object result) {
if (argumentParser.isSuccess() && !TextUtils.isEmpty(argumentParser.getModule())
&& !TextUtils.isEmpty(argumentParser.getMethod())) {
JsModule findModule = getModule(argumentParser.getModule());
if (findModule != null) {
HashMap<String, JsMethod> methodHashMap = exposedMethods.get(findModule);
if (methodHashMap != null && !methodHashMap.isEmpty() && methodHashMap.containsKey(
argumentParser.getMethod())) {
// Issue #25: report routing failures with the actual cause
// instead of a generic "JBArgument Parse error" which makes
// every developer think their JSON is malformed.
if (findModule == null) {
String msg = "Module not found: " + argumentParser.getModule();
JBLog.e(msg, null);
setJsPromptResult(result, false, msg);
return true;
}
HashMap<String, JsMethod> methodHashMap = exposedMethods.get(findModule);
if (methodHashMap == null || methodHashMap.isEmpty()
|| !methodHashMap.containsKey(argumentParser.getMethod())) {
String msg = "Method not found: " + argumentParser.getModule()
+ "." + argumentParser.getMethod();
JBLog.e(msg, null);
setJsPromptResult(result, false, msg);
return true;
}
if (true) {
JsMethod method = methodHashMap.get(argumentParser.getMethod());
List<JBArgumentParser.Parameter> parameters = argumentParser.getParameters();
Comment on lines +294 to 296
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
int length = method.getParameterType().size();
Expand Down Expand Up @@ -327,9 +342,6 @@ private boolean onCallJsPrompt(String methodArgs, Object result) {
}
return true;
}
}
setJsPromptResult(result, false, "JBArgument Parse error");
return true;
}
JBLog.e("JBArgument error", argumentParser.getThrowable());
setJsPromptResult(result, false, argumentParser.getErrorMsg());
Expand Down
Loading