Skip to content

Don't crash when upgrading a gecko profile with an empty sample_groups#6160

Merged
canova merged 2 commits into
firefox-devtools:mainfrom
spokodev:fix/profile-logic
Jul 10, 2026
Merged

Don't crash when upgrading a gecko profile with an empty sample_groups#6160
canova merged 2 commits into
firefox-devtools:mainfrom
spokodev:fix/profile-logic

Conversation

@spokodev

@spokodev spokodev commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Main | Deploy preview

The version 29 gecko upgrader unwraps counter.sample_groups[0].samples into counter.samples, guarding only against a missing sample_groups:

if (!counter.sample_groups) {
  continue;
}
counter.samples = counter.sample_groups[0].samples;

But the version 18 upgrader deliberately produces an empty array for a counter that collected no data:

// It's possible to have an empty sample_groups object due to gecko bug.
// Remove it if that's the case.
if ('samples' in counter.sample_groups) {
  counter.sample_groups = [counter.sample_groups];
} else {
  counter.sample_groups = [];
}

[] is truthy, so the !counter.sample_groups guard doesn't skip it, and [][0].samples throws TypeError: Cannot read properties of undefined (reading 'samples'), aborting the whole profile load. No upgrader between 19 and 28 touches counters, so the empty array reaches version 29 unchanged. Any gecko profile from version 17–28 that contains a data-less counter (e.g. an external power counter that recorded nothing) hits this.

Fix: when there's no group to unwrap, fall back to an empty samples table. Downstream _processCounters already drops counters whose samples.data is empty, so the counter is handled gracefully.

Added a regression test that upgrades a version 17 profile with an empty sample_groups object to the current version; it fails on the current code with the TypeError above and passes with the fix.

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.50%. Comparing base (625c2f6) to head (6d49630).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6160   +/-   ##
=======================================
  Coverage   83.50%   83.50%           
=======================================
  Files         342      342           
  Lines       36522    36525    +3     
  Branches    10125    10224   +99     
=======================================
+ Hits        30497    30502    +5     
+ Misses       5597     5595    -2     
  Partials      428      428           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mstange mstange requested a review from fatadel July 9, 2026 16:29

@canova canova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@canova canova enabled auto-merge (squash) July 10, 2026 11:01
@canova canova merged commit 98f30bf into firefox-devtools:main Jul 10, 2026
21 checks passed
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.

2 participants