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

Fix: HostApp is crashing with NoSuchFieldException in staging #149

Merged

Conversation

khairul-alam-licon
Copy link
Contributor

@khairul-alam-licon khairul-alam-licon commented Sep 15, 2020

Description

HostApp is crashing in staging with following exception when requesting custom permission e.g. USER_NAME

java.lang.AssertionError: java.lang.NoSuchFieldException: USER_NAME
        at com.google.gson.internal.bind.TypeAdapters$EnumTypeAdapter.<init>(:792)
        at com.google.gson.internal.bind.TypeAdapters$30.a(:818)
        at com.google.gson.Gson.k(:458)
       ......
Caused by: java.lang.NoSuchFieldException: USER_NAME
        at java.lang.Class.getField(Class.java:1601)
        at com.google.gson.internal.bind.TypeAdapters$EnumTypeAdapter.<init

This is not happening in debug. After reading few issues e.g. google/gson#1776 and google/gson#924, I have come out with two solutions:

  • Solution-1: Set minifyEnabled false in here as the staging buildTypes takes some favors from release.
  • Solution-2: Add -keepclassmembers enum * { *; } in miniapp/proguard-rules.pro
  • Solution-3: Use @Keep annotation as @minh-rakuten suggested.

Links

MINI-1690

Checklist

  • I have read the contributing guidelines.
  • I wrote/updated tests for new/changed code
  • I removed all sensitive data before every commit, including API endpoints and keys
  • I ran ./gradlew check without errors

@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2020

Codecov Report

Merging #149 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #149   +/-   ##
=======================================
  Coverage   89.58%   89.58%           
=======================================
  Files          33       33           
  Lines         768      768           
  Branches       68       68           
=======================================
  Hits          688      688           
  Misses         55       55           
  Partials       25       25           
Impacted Files Coverage Δ
...ech/mobile/miniapp/permission/MiniAppPermission.kt 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8618128...c986454. Read the comment docs.

Copy link
Contributor

@minh-rakuten minh-rakuten left a comment

Choose a reason for hiding this comment

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

You can fix it by putting @Keep in enum class. Those enum classes are in MiniAppPermission.kt. You should check the data class of MiniAppCustomPermission too.

@@ -1,5 +1,7 @@
package com.rakuten.tech.mobile.miniapp.permission

import androidx.annotation.Keep

/** Type of miniapp permission. **/
enum class MiniAppPermissionType(val type: String) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this one also need @Keep?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, there is no affect has been traced for this class without @Keep. But for safety we can address it here, what do you think @minh-rakuten?

Copy link
Contributor

Choose a reason for hiding this comment

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

@khairul-alam-licon I guess just putting @Keep on data class MiniAppCustomPermission will help you fix the problem, no need to update @Keep for enum in MiniAppPermission.kt. I'm not sure so let try it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@minh-rakuten I understand your concern, as MiniAppCustomPermission has a pairValues. Here, I re-checked that, adding @Keep annotation only in MiniAppCustomPermissionType and MiniAppCustomPermissionResult helps actually. So, I updated this PR. For now, let's keep it to understand clearly where we require this annotation.

Copy link
Contributor

@corycaywood corycaywood left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@minh-rakuten minh-rakuten left a comment

Choose a reason for hiding this comment

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

👍 👍

@khairul-alam-licon
Copy link
Contributor Author

Thanks guys for the reviews 👍

@khairul-alam-licon khairul-alam-licon merged commit 4ffea90 into rakutentech:master Sep 15, 2020
@khairul-alam-licon khairul-alam-licon deleted the fix_no_field_stg branch September 15, 2020 07:03
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.

None yet

4 participants