Skip to content
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

Merged
merged 2 commits into from
Jul 4, 2024

Conversation

atavism
Copy link
Contributor

@atavism atavism commented Jul 4, 2024

No description provided.

@@ -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())
Copy link
Contributor

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.

Copy link
Contributor Author

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

Copy link
Contributor

@jigar-f jigar-f left a 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

@atavism atavism merged commit 8b99194 into atavism/prouser-default-values Jul 4, 2024
@atavism atavism deleted the atavism/try-parse-json-util branch July 4, 2024 16:50
atavism added a commit that referenced this pull request Jul 4, 2024
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants