bug: [CapacitorHttp] error parsing certain JSON responses

[JanMisker]

Bug Report

Capacitor Version

Latest Dependencies:

  @capacitor/cli: 4.6.1
  @capacitor/core: 4.6.1
  @capacitor/android: 4.6.1
  @capacitor/ios: 4.6.1

Installed Dependencies:

  @capacitor/cli: 4.6.1
  @capacitor/core: 4.6.1
  @capacitor/android: 4.6.1
  @capacitor/ios: 4.6.1

[success] iOS looking great! 👌
[success] Android looking great! 👌

Platform(s)

Checked it only on iOS for now.

Current Behavior

I have the CapacitorHttp plugin enabled.
The following response body is not parsed as JSON but instead returns the string The data couldn’t be read because it isn’t in the correct format.

"abc"

I suppose it is debatable whether this is valid JSON, it's of course not an object but a string.
However the generic failure message feels a bit too much like a "bail out".

Expected Behavior

Replacing the string The data couldn’t be read because it isn’t in the correct format. with the actual text of the response data would help a lot.
This can easily be achieved by replacing return error.localizedDescription with return String(data: data, encoding: .utf8), maybe with another check to remove the " quotes.

capacitor/ios/Capacitor/Capacitor/Plugins/HttpRequestHandler.swift

Lines 32 to 38 in f4e7f19

	func tryParseJson(_ data: Data) -> Any {
	do {
	return try JSONSerialization.jsonObject(with: data, options: .mutableContainers)
	} catch {
	return error.localizedDescription
	}
	}

Code Reproduction

Bit tricky to make a repo, as it depends on the server output.

Other Technical Details

Additional Context

[JanMisker]

I just checked it on Android as well. There the result is also a generic message "JSONException".
Looking at the source I do see there are some other cases that are taken into account, null, true, and false. But also here return the string would be more useful than the generic error message.

capacitor/android/capacitor/src/main/java/com/getcapacitor/plugin/util/HttpRequestHandler.java

Lines 297 to 318 in f4e7f19

	private static Object parseJSON(String input) throws JSONException {
	JSONObject json = new JSONObject();
	try {
	if ("null".equals(input.trim())) {
	return JSONObject.NULL;
	} else if ("true".equals(input.trim())) {
	return new JSONObject().put("flag", "true");
	} else if ("false".equals(input.trim())) {
	return new JSONObject().put("flag", "false");
	} else if (input.trim().length() <= 0) {
	return "";
	} else {
	try {
	return new JSObject(input);
	} catch (JSONException e) {
	return new JSArray(input);
	}
	}
	} catch (JSONException e) {
	return new JSArray(input);
	}
	}

[silviogutierrez]

@JanMisker : happening as well in my code. "Real" fetch works correctly, but the patched fetch fails on anything that isn't a JSON array or object pretty much.

I realize replacing "fetch" seamlessly is a Herculean task. But the number of bugs in this feature is staggering.

[Ionitron]

This issue may need more information before it can be addressed. In particular, it will need a reliable Code Reproduction that demonstrates the issue.

Please see the Contributing Guide for how to create a Code Reproduction.

Thanks!
Ionitron 💙

[silviogutierrez]

@JanMisker : I tried to work around this issue by calling response.text() and manually parsing the JSON, but Capacitor looks at the header and gives the header precedence over the text() call, so this will still return the localized string indicating invalid JSON. I filed #6184 to track , as technically it's a separate issue.

As for this part:

I suppose it is debatable whether this is valid JSON, it's of course not an object but a string.

It is valid JSON, see https://www.json.org/json-en.html, so it's part of the spec.

[JanMisker]

@silviogutierrez nice to see you're also busy with it.
I've also been looking into this more, your message prompted me to put my changes in code. I didn't check the Android part much yet, but here is a start for a PR: https://github.com/JanMisker/capacitor

I also made this mock response that outputs the JSON string "hello world" with content-type application/json https://run.mocky.io/v3/cb25a9f8-1ef5-4462-8e5a-fc56de26b876

[silviogutierrez]

@JanMisker : looks like for iOS, JSONDecoder will handle this natively. But it requires iOS 13.1+ and I believe Capacitor supports 13.0. See: https://stackoverflow.com/a/59236933

I can't find an equivalent built-in method for Java, but maybe it exists.

I also don't know if there's a reason why the JSON is parsed manually. Maybe something to do with the bridge. Chesterton's fence and all.

What I'm doing now is patching both those codebases to just return a string. Then in my own app I just use JSON.parse().

In fact, you could just manually instantiate new Response(data, {headers, status}) and let JavaScript do the work. Maybe this is an approach worth looking at? Can be done in the bridge.

[JanMisker]

I created a repository to reproduce the error https://github.com/JanMisker/Capacitor-JSONParseError

[JanMisker]

And here is a PR #6224

[bentzibentz]

I have the same problem, the used api endpoint just returns an integer value (e.g. 5). The response returns "The data couldn’t be read because it isn’t in the correct format." instead of the correct value.

Will this be fixed at some point? This prevents you from using CapacitorHttp for a public api where you have no influence on the return value.

[JanMisker]

It's really a pity that there seems to be no interest from the Ionic team, even though this is a core/official plugin.

[markemer]

You've got a PR up? Thanks! I'll take a look.

[markemer]

Fixed in #6224

[ionitron-bot]

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Capacitor, please create a new issue and ensure the template is fully filled out.