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

Assertion thrown in debug builds when generating Brave Ads console log report #8446

Closed
tmancey opened this issue Feb 27, 2020 · 4 comments · Fixed by brave/brave-core#4774
Closed

Comments

@tmancey
Copy link
Contributor

tmancey commented Feb 27, 2020

Description

Assertion thrown in debug builds when generating Brave Ads console log report

Steps to Reproduce

  1. View an ad (in debug build)

Actual result:

Assertion is thrown by RAPIDJSON_ASSERT due to multiple root nodes

Expected result:

Assertion should not be thrown

Reproduces how often:

Easily reproduced (only occurs on first run)

Brave version (brave://version info)

Version/Channel Information:

  • Can you reproduce this issue with the current release? No
  • Can you reproduce this issue with the beta channel? No
  • Can you reproduce this issue with the dev channel? No
  • 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? N/A
  • Is the issue reproducible on the latest version of Chrome? N/A

Miscellaneous Information:

@LaurenWags
Copy link
Member

@tmancey QA doesn't generally test debug builds, we get release builds via github. Is there something that can be checked on a release build? if not, should this one be marked QA/No?

cc @kjozwiak

@kjozwiak
Copy link
Member

We should label this as QA/No as we don't generally check crashes/assertions on debug builds that are built from source as per #8446 (comment). @tmancey if there's nothing that needs to be done here, can you please label this as QA/No? Or just let us know and someone from QA will change the label as needed.

@tmancey
Copy link
Contributor Author

tmancey commented Mar 31, 2020

@LaurenWags @kjozwiak We just need to make sure the fix works, in that we log the correct information to the console.

See PR which has the following test:

Confirm "restart" appears within an AdsService Event Log: in the console log when viewing Brave Ad notifications, i.e.

AdsService Event Log: {"data":{"type":"restart","timestamp":"2020-03-31T10:08:26Z"},"data":{"type":"notify","timestamp":"2020-03-31T10:08:26Z","eventType":"clicked","classifications":["personal finance"],"adCatalog":"37f7ba07-355d-4d0f-8cdb-b9241550c0c8","targetUrl":"https://travala.com/"}}

Note you must also pass --v=1 to see AdsService Event Log's

@LaurenWags
Copy link
Member

LaurenWags commented Mar 31, 2020

Verified passed with

Brave 1.7.79 Chromium: 80.0.3987.149 (Official Build) dev (64-bit)
Revision 5f4eb224680e5d7dca88504586e9fd951840cac6-refs/branch-heads/3987_137@{#16}
OS macOS Version 10.14.6 (Build 18G3020)
[11515:775:0331/094928.107220:INFO:ads_service_impl.cc(2136)] AdsService Event Log: {"data":{"type":"restart","timestamp":"2020-03-31T09:49:28Z"},"data":{"type":"notify","timestamp":"2020-03-31T09:49:28Z","eventType":"generated","classifications":["technology & computing"],"adCatalog":"61ad680e-d3d4-4e8f-b8af-09046ea0bda2","targetUrl":"https://creators.brave.com/?src=ba6"}}
[11515:775:0331/094932.456062:INFO:ads_service_impl.cc(2136)] AdsService Event Log: {"data":{"type":"restart","timestamp":"2020-03-31T09:49:32Z"},"data":{"type":"notify","timestamp":"2020-03-31T09:49:32Z","eventType":"clicked","classifications":["technology & computing"],"adCatalog":"61ad680e-d3d4-4e8f-b8af-09046ea0bda2","targetUrl":"https://creators.brave.com/?src=ba6"}}

Verification passed on

Brave 1.7.79 Chromium: 80.0.3987.149 (Official Build) dev (64-bit)
Revision 5f4eb224680e5d7dca88504586e9fd951840cac6-refs/branch-heads/3987_137@{#16}
OS Ubuntu 18.04 LTS
[27944:27944:0401/045719.432455:INFO:ads_service_impl.cc(2136)] AdsService Event Log: {"data":{"type":"restart","timestamp":"2020-04-01T04:57:19Z"},"data":{"type":"notify","timestamp":"2020-04-01T04:57:19Z","eventType":"generated","classifications":["arts & entertainment"],"adCatalog":"b2054163-9998-4b88-aa0c-9939f55d414a","targetUrl":"https://travala.com/"}}
[27944:27944:0401/045721.989294:INFO:ads_service_impl.cc(2136)] AdsService Event Log: {"data":{"type":"restart","timestamp":"2020-04-01T04:57:21Z"},"data":{"type":"notify","timestamp":"2020-04-01T04:57:21Z","eventType":"dismissed","classifications":["arts & entertainment"],"adCatalog":"b2054163-9998-4b88-aa0c-9939f55d414a","targetUrl":"https://travala.com/"}}

Verification passed on

Brave 1.7.79 Chromium: 80.0.3987.149 (Official Build) dev (64-bit)
Revision 5f4eb224680e5d7dca88504586e9fd951840cac6-refs/branch-heads/3987_137@{#16}
OS Windows 10 OS Version 1803 (Build 17134.1006)
[7740:10304:0401/104405.973:INFO:ads_service_impl.cc(2136)] AdsService Event Log: {"data":{"type":"restart","timestamp":"2020-04-01T10:44:05Z"},"data":{"type":"notify","timestamp":"2020-04-01T10:44:05Z","eventType":"generated","classifications":["arts & entertainment","video games"],"adCatalog":"b50a77bb-edb2-477d-a9d2-eca42e36d6a8","targetUrl":"https://www.godsunchained.com?utm_source=brave&utm_medium=display&utm_campaign=gu_brave_test_mar2020&utm_content=paidawareness&placement_name=desktop"}}

[7740:10304:0401/104410.748:INFO:ads_service_impl.cc(2136)] AdsService Event Log: {"data":{"type":"restart","timestamp":"2020-04-01T10:44:10Z"},"data":{"type":"notify","timestamp":"2020-04-01T10:44:10Z","eventType":"clicked","classifications":["arts & entertainment","video games"],"adCatalog":"b50a77bb-edb2-477d-a9d2-eca42e36d6a8","targetUrl":"https://www.godsunchained.com?utm_source=brave&utm_medium=display&utm_campaign=gu_brave_test_mar2020&utm_content=paidawareness&placement_name=desktop"}}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants