-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Make tipping banner response depend on success/error from server: Follow up to 15543 #19071
Comments
thanks for the updates to this issue @Miyayes 👍🏻 |
Will be able to implement fix following this refactor: #20473. |
I think there's a good chance @zenparsing's work on refactoring the |
Possible workaround w/o full contribution engine refactor: Tipping banner won't wait for contribution engine. It'll get a success response from contribution engine ("successfully put into queue"), but then it would listen for general contribution failures. @zenparsing |
Verified with
Note, manually enabled "Use WebUI Rewards Panel" prior to below cases to do additional testing of #22423. Per the test plan in brave/brave-core#14205 (comment), the following items have been regression tested via #20748 (comment):
vBAT failed tip - FAILED, follow up issue loggedVerified STR from Case 1 in #19071 (comment). Failed as tip banner still showed success even though logs showed 500. Tip also went into retry loop (not sure if this part is expected). Follow up issue logged: #24707 Logs:
Uphold failed tip - PASSEDVerified STR from Case 2 in #19071 (comment). Confirmed when Logs:
Gemini failed tip - PASSEDVerified STR from Case 2 in #19071 (comment) for Gemini. Confirmed when Logs:
bitFlyer failed tip - PASSEDVerified STR from Case 2 in #19071 (comment) for bitFlyer. Confirmed when Logs:
Note, manually enabled "Use WebUI Rewards Panel" prior to below cases to do additional testing of #22423. Per the test plan in brave/brave-core#14205 (comment), the following items have been regression tested:
|
Verification PASSED on
Uphold failed tip - PASSEDVerified STR from Case 2 in #19071 (comment). Confirmed when https://api-sandbox.uphold.com/v0/me/cards/card-id/transactions returns 500, the tip banner reflects that there is a failed tip. Logs:
|
Description
Update Oct 29, 2021: Upon further testing and investigation, original #15543 was not resolved by changes there. Much deeper changes to the contribution engine will be needed to successfully resolve #15543. I have updated title of this issue to become the new #15543.
@szilardszaloki found this issue while discussing #15543
Due to a long existing but, anon or Uphold user wallets will always show "Success" (confetti) after tipping. They do not show the "Failure" state. From @szilardszaloki:
Just found a bug deep down in the contribution code (which has been around since June 2020).
ContributionTip
doesn't pass down the callback to the layer that actually handles the processing of contributions, but instead returns withLEDGER_OK
when saving the contribution queue in thecontribution_queue
table (no matter if it was successful or not).Steps to Reproduce
Case 1 - Anon wallet (pre-req: have Charles Proxy set up to return 500 error for
https://grant.rewards.bravesoftware.com/v1/suggestions
)Case 2 - Uphold wallet (pre-req: have Charles Proxy set up to return 500 error for
https://api-sandbox.uphold.com/v0/me/cards/card-id/transactions
)All tipping scenarios for all wallet types (including bitFlyer and Gemini) would need to be retested once this is resolved.
Actual result:
Success on tip banner
Expected result:
Failure on tip banner
Reproduces how often:
easily
Brave version (brave://version info)
Version/Channel Information:
Other Additional Information:
Miscellaneous Information:
cc @szilardszaloki @Miyayes
The text was updated successfully, but these errors were encountered: