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

Properly check for empty stats updater URL in GN #7789

Merged
merged 1 commit into from
Feb 3, 2021

Conversation

bsclifton
Copy link
Member

@bsclifton bsclifton commented Feb 2, 2021

Fixes brave/brave-browser#13858

Original fixes:

Submitter Checklist:

  • There is a ticket for my issue.
  • Used Github auto-closing keywords in the commit message.
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed).
  • Requested a security/privacy review as needed.

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

See brave/brave-browser#13858

@bsclifton bsclifton self-assigned this Feb 2, 2021
@bsclifton bsclifton requested a review from kkuehlz February 2, 2021 20:24
@bsclifton bsclifton force-pushed the bsc-stats-url-crash-fix branch from 09a96eb to 9c880f7 Compare February 2, 2021 20:28
@bsclifton bsclifton force-pushed the bsc-stats-url-crash-fix branch from 9c880f7 to a0ea13e Compare February 2, 2021 20:55
- check stats updater URL in GN to fail quick.
- remove check which causes crash since GN check is done.

Fixes brave/brave-browser#13858
@bsclifton bsclifton force-pushed the bsc-stats-url-crash-fix branch from a0ea13e to efc05f0 Compare February 2, 2021 21:01
@bsclifton
Copy link
Member Author

Only CI failure was on Windows with [ FAILED ] AdBlockServiceTest.CosmeticFilteringProtect1p which is a known issue captured with brave/brave-browser#13887

Good to go 👍

@bsclifton bsclifton merged commit c21ddd9 into master Feb 3, 2021
@bsclifton bsclifton deleted the bsc-stats-url-crash-fix branch February 3, 2021 05:42
@bsclifton bsclifton added this to the 1.21.x - Nightly milestone Feb 3, 2021
@bsclifton bsclifton changed the title Only crash for empty stats_updater_url on release Properly check for empty stats updater URL in GN Feb 3, 2021
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.

nightly crashing due to "brave_stats::BraveStatsUpdater::BuildStatsEndpoint"
3 participants