Skip to content

fix(performanceTimer): throwing in axe catch clause#4852

Merged
WilcoFiers merged 2 commits intodevelopfrom
perf-measure-catch
Aug 11, 2025
Merged

fix(performanceTimer): throwing in axe catch clause#4852
WilcoFiers merged 2 commits intodevelopfrom
perf-measure-catch

Conversation

@WilcoFiers
Copy link
Copy Markdown
Contributor

@WilcoFiers WilcoFiers commented Aug 8, 2025

I ran into a problem with this:

axe-core/lib/core/base/rule.js

Lines 297 to 300 in e7dae4e

if (options.performanceTimer) {
this._logRulePerformance();
}
reject(error);

If the end mark isn't set, performance.measure throws an error. Since we call performance.measure in the catch, it is quite possible that the end mark doesn't exist. And then throwing from a catch completely throws the run. I don't think axe should fail if the performance measure fails, so instead I'm going to have it just log the error and move forward.

Copilot AI review requested due to automatic review settings August 8, 2025 15:29
@WilcoFiers WilcoFiers requested a review from a team as a code owner August 8, 2025 15:29
Copilot AI reviewed Aug 8, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a critical issue where the performanceTimer.measure method would throw an error if performance marks don't exist, which could cause the entire axe-core run to fail when called from catch blocks. The fix wraps the performance.measure call in a try-catch block to log errors instead of propagating them.

  • Added error handling to prevent performance measurement failures from breaking axe runs
  • Added comprehensive test coverage for error scenarios in the measure method

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
lib/core/utils/performance-timer.js Added try-catch wrapper around performance.measure to log errors instead of throwing
test/core/utils/performance-timer.js Added test cases for error handling when performance marks are missing

lib/core/utils/performance-timer.js
try {
window.performance.measure(measureName, startMark, endMark);
} catch (e) {
this._log(e);
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

Logging the raw error object may not provide useful information to users. Consider logging a more descriptive error message like this._log('Performance measurement failed: ' + e.message); to make the logged error more helpful.

Suggested change
this._log(e);
this._log('Performance measurement failed: ' + (e && e.message ? e.message : e));

Copilot uses AI. Check for mistakes.
@WilcoFiers WilcoFiers merged commit a4ade04 into develop Aug 11, 2025
23 checks passed
@WilcoFiers WilcoFiers deleted the perf-measure-catch branch August 11, 2025 16:00
@attest-team-ci attest-team-ci mentioned this pull request Oct 7, 2025
Merged
straker added a commit that referenced this pull request Oct 9, 2025
@straker
chore(release): v4.11.0 (#4904)
7dc4242 
##
[4.11.0](v4.10.3...v4.11.0)
(2025-10-07)

### Features

- add RGAA tags to rules
([#4862](#4862))
([53a925a](53a925a))
- **aria-prohibited-attr:** add support for fallback roles
([#4325](#4325))
([62a19a9](62a19a9))
- **axe.d.ts:** add nodeSerializer typings
([#4551](#4551))
([a2f3a48](a2f3a48)),
closes [#4093](#4093)
- **DqElement:** deprecate fromFrame function
([#4881](#4881))
([374c376](374c376)),
closes [#4093](#4093)
- **DqElement:** Truncate large `html` strings when the element has a
large outerHTML string
([#4796](#4796))
([404a4fb](404a4fb)),
closes [#4544](#4544)
- **get-xpath:** return proper relative selector for id
([#4846](#4846))
([1035f9e](1035f9e)),
closes [#4845](#4845)
- **i18n:** Add Portugal Portuguese translation
([#4725](#4725))
([5b6a65a](5b6a65a))
- incomplete with node on which an error occurred
([#4863](#4863))
([32ed8da](32ed8da))
- **locale:** Added ru locale
([#4565](#4565))
([067b01d](067b01d))
- **tap:** some best practice rules map to RGAA
([#4895](#4895))
([bc33f4c](bc33f4c))
- **td-headers-attr:** report headers attribute referencing other <td>
elements as unsupported
([#4589](#4589))
([ec7c6c8](ec7c6c8)),
closes [#3987](#3987)

### Bug Fixes

- **aria-allowed-role:** add form to allowed roles of form element
([#4588](#4588))
([8aa47ac](8aa47ac)),
closes
[/github.com/dequelabs/axe-core/blob/develop/lib/standards/html-elms.js#L264](https://github.com/dequelabs//github.com/dequelabs/axe-core/blob/develop/lib/standards/html-elms.js/issues/L264)
- **aria-allowed-role:** Add math to allowed roles for img element
([#4658](#4658))
([95b6c18](95b6c18)),
closes [#4657](#4657)
- **autocomplete-valid :** Ignore readonly autocomplete field
([#4721](#4721))
([491f4ec](491f4ec)),
closes [#4708](#4708)
- **autocomplete-valid:** treat values "xon" and "xoff" as
non-WCAG-violations
([#4878](#4878))
([52bc611](52bc611)),
closes [#4877](#4877)
- **axe.d.ts:** add typings for preload options object
([#4543](#4543))
([cfd2974](cfd2974))
- **button-name,input-button-name,input-img-alt:** allow label to give
accessible name
([#4607](#4607))
([a9710d7](a9710d7)),
closes [#4472](#4472)
[#3696](#3696)
[#3696](#3696)
- **captions:** fix grammar in captions check incomplete message
([#4661](#4661))
([11de515](11de515))
- **color-contrast:** do not run on elements with font-size: 0
([#4822](#4822))
([d77c885](d77c885)),
closes [#4820](#4820)
- consistently parse tabindex, following HTML 5 spec
([#4637](#4637))
([645a850](645a850)),
closes [#4632](#4632)
- **core:** measure perf for async checks
([#4609](#4609))
([7e9bacf](7e9bacf))
- fix grammar when using "alternative text" in a sentence
([#4811](#4811))
([237a586](237a586)),
closes [#4394](#4394)
- **get-ancestry:** add nth-child selector for multiple siblings of
shadow root ([#4606](#4606))
([1cdd6c3](1cdd6c3)),
closes [#4563](#4563)
- **get-ancestry:** don't error when there is no parent
([#4617](#4617))
([a005703](a005703))
- **locale:** fix typos in japanese (ja) locale
([#4856](#4856))
([3462cc5](3462cc5))
- **locale:** fixed typos in german (DE) locale
([#4631](#4631))
([b7736de](b7736de))
- **locale:** proofread and updated de.json
([#4643](#4643))
([8060ada](8060ada))
- **meta-viewport:** lower impact to moderate
([#4887](#4887))
([2f32aa5](2f32aa5)),
closes [#4714](#4714)
- **no-autoplay-audio:** don't timeout for preload=none media elements
([#4684](#4684))
([cdc871e](cdc871e))
- **performanceTimer:** throwing in axe catch clause
([#4852](#4852))
([a4ade04](a4ade04)),
closes
[/github.com/dequelabs/axe-core/blob/e7dae4ec48cbfef74de9f833fdcfb178c1002985/lib/core/base/rule.js#L297-L300](https://github.com/dequelabs//github.com/dequelabs/axe-core/blob/e7dae4ec48cbfef74de9f833fdcfb178c1002985/lib/core/base/rule.js/issues/L297-L300)
- **performanceTimer:** work in frames
([#4834](#4834))
([d7dfebc](d7dfebc))
- **rules:** Change "alternate text" to "alternative text"
([#4582](#4582))
([b03ada3](b03ada3))
- **target-size:** do not treat focusable tabpanels as targets
([#4702](#4702))
([60d11f2](60d11f2)),
closes [#4421](#4421)
[#4701](#4701)
- **type:** correct RuleError type
([#4893](#4893))
([d1aa8e2](d1aa8e2))
- **types:** correct raw types
([#4903](#4903))
([3eade11](3eade11))

This PR was opened by a robot 🤖 🎉
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@straker straker straker approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@WilcoFiers @straker