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

Issue with Ads Payments (August 5) #5548

Closed
mandar-brave opened this issue Aug 6, 2019 · 2 comments · Fixed by brave/brave-core#3098
Closed

Issue with Ads Payments (August 5) #5548

mandar-brave opened this issue Aug 6, 2019 · 2 comments · Fixed by brave/brave-core#3098

Comments

@mandar-brave
Copy link

mandar-brave commented Aug 6, 2019

Description

There is an issue with collecting ads confirmations leaving the browser in some cases not being paid;

First issue: the browser currently only retries sending a failed confirmation per session

Second issue: potential for a race condition; poor interleaving of http responses and multiple confirmations due to network latency, etc may cause r the same token to be re-used for multiple confirmations which will always fail when sent to the server (causes 404).

Steps to Reproduce

  1. See test cases in PR

Actual result:

Failed Ad confirmations do not retry and fail due to a race condition where the same token is used when creating confirmations

Expected result:

Failed Ad confirmations should retry until successful

Reproduces how often:

Intermitten issue

Brave version (brave://version info)

All

Version/Channel Information:

  • Can you reproduce this issue with the current release? Yes
  • Can you reproduce this issue with the beta channel? Yes
  • Can you reproduce this issue with the dev channel? Yes
  • Can you reproduce this issue with the nightly channel? Yes

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? N/A
  • Does the issue resolve itself when disabling Brave Rewards? No
  • Is the issue reproducible on the latest version of Chrome? N/A

Miscellaneous Information:

@mandar-brave mandar-brave added feature/rewards priority/P1 A very extremely bad problem. We might push a hotfix for it. labels Aug 6, 2019
@tmancey tmancey added this to the 0.70.x - Nightly milestone Aug 6, 2019
@tmancey tmancey self-assigned this Aug 6, 2019
@LaurenWags
Copy link
Member

LaurenWags commented Aug 6, 2019

able to reproduce the 400/404's once using

Brave 0.67.123 Chromium: 76.0.3809.87 (Official Build) (64-bit)
Revision 111fe1e15d5ced26080a7dc239bcfe70f6c49aad-refs/branch-heads/3809@{#967}
OS Mac OS X

but haven't been successful in reproducing since.

tmancey added a commit to brave/brave-core that referenced this issue Aug 6, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
tmancey added a commit to brave/brave-core that referenced this issue Aug 7, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
tmancey added a commit to brave/brave-core that referenced this issue Aug 7, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
tmancey added a commit to brave/brave-core that referenced this issue Aug 7, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
tmancey added a commit to brave/brave-core that referenced this issue Aug 7, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
tmancey added a commit to brave/brave-core that referenced this issue Aug 7, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
samartnik pushed a commit to brave/brave-core that referenced this issue Aug 7, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
tmancey added a commit to brave/brave-core that referenced this issue Aug 9, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
tmancey added a commit to brave/brave-core that referenced this issue Aug 9, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
tmancey added a commit to brave/brave-core that referenced this issue Aug 9, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
@LaurenWags
Copy link
Member

LaurenWags commented Aug 12, 2019

Verified passed with

Brave 0.67.125 Chromium: 76.0.3809.100 (Official Build) (64-bit)
Revision ed9d447d30203dc5069e540f05079e493fc1c132-refs/branch-heads/3809@{#990}
OS Mac OS X
  1. PASS Verified no 404s observed in logs when clicking to view ads notifications.
  2. PASS Used Charles Proxy to emulate failed confirmations. Verified that when the failed confirmations were retried, no 400s were observed in the logs.
  3. PASS Emulated failed confirmations on 0.67.124 (current release build) with Charles Proxy. Upgraded on test channel to 0.67.125 rc1. Verified The existing failed confirmations were retried every ~5min. Additionally, Estimated pending rewards and Ad notifications received this month totals were updated as the failed confirmations succeeded (where applicable). Verified that failed confirmations for view, click, dismiss, and landed were each retried and processed successfully.
  4. PASS Verified that failed confirmations were retried every 5 minutes (+jitter) until the queue was empty. Additionally, if the retry failed, it was added back to the queue and retried again in ~5 min.
  5. PASS Using 0.67.125rc1, viewed some ads and emulated some failed confirmations (using Charles Proxy) - so that I had a mix of successful and failed confirmations in my confirmations.json file. Without restarting Brave, verified that the failed confirmations were added to the queue and retried. Verified Estimated pending rewards and Ad notifications received this month totals were updated as the failed confirmations succeeded.
  6. PASS Using 0.67.125rc1, emulated some failed confirmations using Charles Proxy. Opened confirmations.json and duplicated the failed confirmations. Verified that the duplicate failed confirmations did not cause Estimated pending rewards and Ad notifications received this month totals to increase. Note - the dupes were removed from my failed_confirmations list as they were retried and failed as expected. Did see Duplicate unblinded payment token message in the logs.
  7. PASS Verified the queue was empty (in confirmations.json) once all failed transactions were processed.

NOTE - there were server issues during testing (503s were experienced). Due to this I encountered #5599 while testing.

Verification passed on

Brave 0.67.125 Chromium: 76.0.3809.100 (Official Build) (64-bit)
Revision ed9d447d30203dc5069e540f05079e493fc1c132-refs/branch-heads/3809@{#990}
OS Windows 10 OS Version 1803 (Build 17134.523)
  • Emulate failed confirmations using Charles proxy, Verified that when failed confirmations are retried the Response status code: 400 did not appear in the chrome_debug.log

  • Verified no Response status code: 404 seen in logs when clicking to view ads notifications

  • Using 0.67.125rc1, viewed some ads and emulated some failed confirmations (using Charles Proxy) so that I had a mix of successful and failed confirmations in my confirmations.json file. Without restarting Brave, verified that the failed confirmations were added to the queue and retried. Verified Estimated pending rewards and Ad notifications received this month totals were updated as the failed confirmations succeeded.

  • Verified the queue was empty (in confirmations.json) once all failed transactions were processed.

samartnik pushed a commit to brave/brave-core that referenced this issue Aug 28, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
samartnik pushed a commit to brave/brave-core that referenced this issue Sep 9, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
samartnik pushed a commit to brave/brave-core that referenced this issue Sep 20, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
samartnik pushed a commit to brave/brave-core that referenced this issue Sep 23, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
samartnik pushed a commit to brave/brave-core that referenced this issue Oct 1, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
samartnik pushed a commit to brave/brave-core that referenced this issue Oct 7, 2019
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants