-
Notifications
You must be signed in to change notification settings - Fork 4
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.
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
Add tryParseJson utility method #1117
Add tryParseJson utility method #1117
Conversation
@@ -65,8 +65,10 @@ open class LanternHttpClient : HttpClient() { | |||
|
|||
override fun onSuccess(response: Response?, result: JsonObject?) { | |||
result?.let { | |||
val user: ProUser = JsonUtil.fromJson<ProUser>(result.toString()) | |||
cb.onSuccess(response!!, user) | |||
val user: ProUser? = JsonUtil.tryParseJson<ProUser?>(result.toString()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@atavism The only issue is here, while parsing JSON, if we get into any issue, I guess it will never return a response and even if it returns a response then it will return a null guess.
With an older approach, if we get into parsing issue, It will still parse some basic values and return results, so app does not crash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thinking with this change is if the API suddenly behaves in some unexpected manner (returns null for some field when it previously always returned an empty array), we should treat that response as invalid and skip over it vs. trying to extract valid fields from it. With either approach, the app shouldn't crash now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@atavism Added one comment, otherwise LGTM
* Set default values on ProUser data class * Update checksum * use gradle 8.4.0 plugin for now * update sentry * use kotlinx.serialization * use kotlinx.serialization * updates to use kotlinx.serialization * Remove old ProPlan * clean-ups, update logging * Update checksum * Updated android-db and messages. * Added more error handling. * Add tryParseJson utility method (#1117) * Add tryParseJson utility method * formatting --------- Co-authored-by: Jigar-f <jigar@getlantern.org>
No description provided.