Skip to content

Fix NoSuchElementException by using poll when accessing serviceRequests#1190

Merged
vegaro merged 3 commits into
mainfrom
nosuchelementexception
Aug 3, 2023
Merged

Fix NoSuchElementException by using poll when accessing serviceRequests#1190
vegaro merged 3 commits into
mainfrom
nosuchelementexception

Conversation

@vegaro

@vegaro vegaro commented Aug 3, 2023

Copy link
Copy Markdown
Member

We are getting reports of this exception

Exception java.util.NoSuchElementException:
  at java.util.AbstractQueue.remove (AbstractQueue.java:117)
  at com.revenuecat.purchases.google.BillingWrapper.executePendingRequests$lambda-2$lambda-1$lambda-0 (BillingWrapper.kt:101)
  at com.revenuecat.purchases.google.BillingWrapper.executePendingRequests (BillingWrapper.kt:101)
  at com.revenuecat.purchases.google.BillingWrapper.onBillingSetupFinished$lambda-25 (BillingWrapper.kt:672)
  at <private>

In this piece of code

private fun executePendingRequests() {
        synchronized(this@BillingWrapper) {
            while (billingClient?.isReady == true && !serviceRequests.isEmpty()) {
                serviceRequests.remove().let { mainHandler.post { it(null) } }
            }
        }
    }

I can't think of a way that's possible since the access to serviceRequests is synchronized. But reports of the exception come only from Android 12, so I am thinking there could be an issue in Android itself

Anyways, I changed the implementation to use poll instead, which instead of throwing a exception, returns null if the queue is empty

@vegaro vegaro added the pr:fix A bug fix label Aug 3, 2023
@vegaro vegaro requested a review from a team August 3, 2023 10:20
@vegaro vegaro requested a review from MarkVillacampa August 3, 2023 10:26
Comment thread purchases/src/main/kotlin/com/revenuecat/purchases/google/BillingWrapper.kt Outdated
@codecov

codecov Bot commented Aug 3, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1190 (752b847) into main (b428869) will increase coverage by 0.00%.
Report is 2 commits behind head on main.
The diff coverage is 80.00%.

@@           Coverage Diff           @@
##             main    #1190   +/-   ##
=======================================
  Coverage   85.92%   85.92%           
=======================================
  Files         181      181           
  Lines        6216     6217    +1     
  Branches      899      900    +1     
=======================================
+ Hits         5341     5342    +1     
  Misses        533      533           
  Partials      342      342           
Files Changed Coverage Δ
.../com/revenuecat/purchases/google/BillingWrapper.kt 83.91% <80.00%> (+0.03%) ⬆️

@vegaro vegaro requested a review from tonidero August 3, 2023 10:49
@vegaro

vegaro commented Aug 3, 2023

Copy link
Copy Markdown
Member Author

@tonidero another approach

@tonidero tonidero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should update the PR name and description, but this looks good!

@vegaro vegaro changed the title Try catch NoSuchElementException Fix NoSuchElementException by using poll when accessing serviceRequests Aug 3, 2023
@vegaro vegaro merged commit b1f7ece into main Aug 3, 2023
@vegaro vegaro deleted the nosuchelementexception branch August 3, 2023 11:24
tonidero pushed a commit that referenced this pull request Aug 10, 2023
**This is an automatic release.**

### Bugfixes
* Fix NoSuchElementException by using poll when accessing
serviceRequests (#1190) via Cesar de la Vega (@vegaro)
### Other Changes
* Debug view: rename package (#1191) via Toni Rico (@tonidero)
* Debugview: Add snapshot tests for debug view using Paparazzi (#1187)
via Toni Rico (@tonidero)
* Debug view: Add offerings section and purchasing capabilities (#1186)
via Toni Rico (@tonidero)
* Debug view: Initial UI + Usage in MagicWeatherCompose (#1075) via Toni
Rico (@tonidero)
* Remove customEntitlementComputation flavor for non-purchases modules
(#1180) via Toni Rico (@tonidero)

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants