feat: add main-thread blocking detection (Long Tasks + LoAF + web Vitals)#166
feat: add main-thread blocking detection (Long Tasks + LoAF + web Vitals)#166
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds main-thread blocking detection to the JavaScript error catcher using the Long Tasks API and Long Animation Frames (LoAF) API. When a blocking entry is detected (task >50 ms), a dedicated Hawk event is sent immediately with blocking details in the context.
Changes:
- Added new
longTasks.tsaddon that sets up PerformanceObservers for Long Tasks and LoAF APIs - Integrated the observers into the Catcher constructor with configurable options
- Added
mainThreadBlockingconfiguration option toHawkInitialSettings
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/javascript/src/addons/longTasks.ts | New addon implementing Long Tasks and LoAF observers with feature detection and data serialization |
| packages/javascript/src/catcher.ts | Integrated main-thread blocking detection into the constructor with callback to send() |
| packages/javascript/src/types/hawk-initial-settings.ts | Added mainThreadBlocking configuration option type definition |
| packages/javascript/package.json | Bumped minor version from 3.2.18 to 3.3.0 for new feature |
| packages/javascript/README.md | Added documentation for main-thread blocking detection feature with configuration examples |
Comments suppressed due to low confidence (2)
packages/javascript/src/addons/longTasks.ts:182
- Using
buffered: truemeans the observer will immediately process all past long task entries that occurred before the observer was set up. If Hawk is initialized late in the page lifecycle on a slow device, this could result in a burst of events being sent all at once, potentially overwhelming the transport or causing rate limiting issues. Consider documenting this behavior or providing an option to disable buffering for users who only want to track tasks from the point of initialization forward.
}).observe({ type: 'longtask', buffered: true });
packages/javascript/src/addons/longTasks.ts:245
- Using
buffered: truemeans the observer will immediately process all past LoAF entries that occurred before the observer was set up. If Hawk is initialized late in the page lifecycle on a slow device, this could result in a burst of events being sent all at once, potentially overwhelming the transport or causing rate limiting issues. Consider documenting this behavior or providing an option to disable buffering for users who only want to track frames from the point of initialization forward.
}).observe({ type: 'long-animation-frame', buffered: true });
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ['renderStart', loaf.renderStart ? Math.round(loaf.renderStart) : null], | ||
| ['styleAndLayoutStart', loaf.styleAndLayoutStart ? Math.round(loaf.styleAndLayoutStart) : null], | ||
| ['firstUIEventTimestamp', loaf.firstUIEventTimestamp ? Math.round(loaf.firstUIEventTimestamp) : null], |
There was a problem hiding this comment.
The truthy check here will incorrectly treat a value of 0 as falsy and return null instead of 0. If styleAndLayoutStart is 0 (which is a valid timestamp), it will be excluded from the serialized data. Consider using an explicit null/undefined check instead: loaf.styleAndLayoutStart != null ? Math.round(loaf.styleAndLayoutStart) : null
| ['renderStart', loaf.renderStart ? Math.round(loaf.renderStart) : null], | |
| ['styleAndLayoutStart', loaf.styleAndLayoutStart ? Math.round(loaf.styleAndLayoutStart) : null], | |
| ['firstUIEventTimestamp', loaf.firstUIEventTimestamp ? Math.round(loaf.firstUIEventTimestamp) : null], | |
| ['renderStart', loaf.renderStart != null ? Math.round(loaf.renderStart) : null], | |
| ['styleAndLayoutStart', loaf.styleAndLayoutStart != null ? Math.round(loaf.styleAndLayoutStart) : null], | |
| ['firstUIEventTimestamp', loaf.firstUIEventTimestamp != null ? Math.round(loaf.firstUIEventTimestamp) : null], |
| new PerformanceObserver((list) => { | ||
| for (const entry of list.getEntries()) { | ||
| const task = entry as LongTaskPerformanceEntry; | ||
| const durationMs = Math.round(task.duration); | ||
| const attr = task.attribution?.[0]; | ||
|
|
||
| const details = compact([ | ||
| ['kind', 'longtask'], | ||
| ['entryName', task.name], | ||
| ['startTime', Math.round(task.startTime)], | ||
| ['durationMs', durationMs], | ||
| ['containerType', attr?.containerType], | ||
| ['containerSrc', attr?.containerSrc], | ||
| ['containerId', attr?.containerId], | ||
| ['containerName', attr?.containerName], | ||
| ]); | ||
|
|
||
| onEntry({ title: `Long Task ${durationMs} ms`, context: { details } }); | ||
| } | ||
| }).observe({ type: 'longtask', buffered: true }); |
There was a problem hiding this comment.
The PerformanceObserver instance created here is not stored or returned, making it impossible to disconnect or clean up the observer later. This could be a memory leak concern if the Hawk catcher is destroyed and recreated multiple times in the application lifecycle (e.g., in SPAs with dynamic module loading). Consider storing the observer instance and providing a cleanup mechanism, similar to how other addons handle resource management.
| if (options.longAnimationFrames ?? true) { | ||
| observeLoAF(onEntry); | ||
| } | ||
| } |
There was a problem hiding this comment.
The new main-thread blocking detection feature lacks test coverage. While testing browser APIs like PerformanceObserver can be challenging, the codebase has comprehensive tests for other addons (e.g., breadcrumbs.test.ts). Consider adding tests for at least: 1) feature detection logic (supportsEntryType), 2) the compact utility function, 3) script serialization (serializeScript), 4) proper handling of callbacks when entries are detected, and 5) handling of disabled options. These can be tested with mocks for PerformanceObserver.
| } | |
| } | |
| /** | |
| * Internal helpers exported for unit testing. | |
| * | |
| * These exports are not part of the public API surface and may change | |
| * without notice. They exist solely to enable targeted tests for: | |
| * - feature detection logic (supportsEntryType) | |
| * - the compact utility function | |
| * - script serialization (serializeScript) | |
| * - observer behavior and option handling | |
| */ | |
| // eslint-disable-next-line @typescript-eslint/naming-convention | |
| export const __longTasksTestables__ = { | |
| supportsEntryType, | |
| compact, | |
| serializeScript, | |
| observeLongTasks, | |
| observeLoAF, | |
| observeMainThreadBlocking, | |
| }; |
| ['sourceCharPosition', s.sourceCharPosition != null && s.sourceCharPosition >= 0 ? s.sourceCharPosition : null], | ||
| ['duration', Math.round(s.duration)], | ||
| ['executionStart', s.executionStart != null ? Math.round(s.executionStart) : null], | ||
| ['forcedStyleAndLayoutDuration', s.forcedStyleAndLayoutDuration ? Math.round(s.forcedStyleAndLayoutDuration) : null], |
There was a problem hiding this comment.
The truthy check here will incorrectly treat a value of 0 as falsy and return null instead of 0. If forcedStyleAndLayoutDuration is 0 (which is a valid duration meaning no forced style/layout work), it will be excluded from the serialized data. Consider using an explicit null/undefined check instead: s.forcedStyleAndLayoutDuration != null ? Math.round(s.forcedStyleAndLayoutDuration) : null
| ['forcedStyleAndLayoutDuration', s.forcedStyleAndLayoutDuration ? Math.round(s.forcedStyleAndLayoutDuration) : null], | |
| ['forcedStyleAndLayoutDuration', s.forcedStyleAndLayoutDuration != null ? Math.round(s.forcedStyleAndLayoutDuration) : null], |
| ['blockingDurationMs', blockingDurationMs], | ||
| ['renderStart', loaf.renderStart ? Math.round(loaf.renderStart) : null], | ||
| ['styleAndLayoutStart', loaf.styleAndLayoutStart ? Math.round(loaf.styleAndLayoutStart) : null], | ||
| ['firstUIEventTimestamp', loaf.firstUIEventTimestamp ? Math.round(loaf.firstUIEventTimestamp) : null], |
There was a problem hiding this comment.
The truthy check here will incorrectly treat a value of 0 as falsy and return null instead of 0. If firstUIEventTimestamp is 0 (which is theoretically a valid timestamp at the navigation start), it will be excluded from the serialized data. Consider using an explicit null/undefined check instead: loaf.firstUIEventTimestamp != null ? Math.round(loaf.firstUIEventTimestamp) : null
| ['firstUIEventTimestamp', loaf.firstUIEventTimestamp ? Math.round(loaf.firstUIEventTimestamp) : null], | |
| ['firstUIEventTimestamp', loaf.firstUIEventTimestamp != null ? Math.round(loaf.firstUIEventTimestamp) : null], |
| new PerformanceObserver((list) => { | ||
| for (const entry of list.getEntries()) { | ||
| const loaf = entry as LoAFEntry; | ||
| const durationMs = Math.round(loaf.duration); | ||
| const blockingDurationMs = loaf.blockingDuration != null | ||
| ? Math.round(loaf.blockingDuration) | ||
| : null; | ||
|
|
||
| const relevantScripts = loaf.scripts?.filter((s) => s.sourceURL || s.sourceFunctionName); | ||
|
|
||
| const scripts = relevantScripts?.length | ||
| ? relevantScripts.reduce<Json>((acc, s, i) => { | ||
| acc[`script_${i}`] = serializeScript(s); | ||
|
|
||
| return acc; | ||
| }, {}) | ||
| : null; | ||
|
|
||
| const details = compact([ | ||
| ['kind', 'loaf'], | ||
| ['startTime', Math.round(loaf.startTime)], | ||
| ['durationMs', durationMs], | ||
| ['blockingDurationMs', blockingDurationMs], | ||
| ['renderStart', loaf.renderStart ? Math.round(loaf.renderStart) : null], | ||
| ['styleAndLayoutStart', loaf.styleAndLayoutStart ? Math.round(loaf.styleAndLayoutStart) : null], | ||
| ['firstUIEventTimestamp', loaf.firstUIEventTimestamp ? Math.round(loaf.firstUIEventTimestamp) : null], | ||
| ['scripts', scripts], | ||
| ]); | ||
|
|
||
| const blockingNote = blockingDurationMs != null | ||
| ? ` (blocking ${blockingDurationMs} ms)` | ||
| : ''; | ||
|
|
||
| const topScript = relevantScripts?.[0]; | ||
| const culprit = topScript?.sourceFunctionName | ||
| || topScript?.invoker | ||
| || topScript?.sourceURL | ||
| || ''; | ||
| const culpritNote = culprit ? ` — ${culprit}` : ''; | ||
|
|
||
| onEntry({ | ||
| title: `Long Animation Frame ${durationMs} ms${blockingNote}${culpritNote}`, | ||
| context: { details }, | ||
| }); | ||
| } | ||
| }).observe({ type: 'long-animation-frame', buffered: true }); |
There was a problem hiding this comment.
The PerformanceObserver instance created here is not stored or returned, making it impossible to disconnect or clean up the observer later. This could be a memory leak concern if the Hawk catcher is destroyed and recreated multiple times in the application lifecycle (e.g., in SPAs with dynamic module loading). Consider storing the observer instance and providing a cleanup mechanism, similar to how other addons handle resource management.
| /** | ||
| * Build a Json object from entries, dropping null / undefined / empty-string values | ||
| */ | ||
| function compact(entries: [string, JsonNode | null | undefined][]): Json { |
There was a problem hiding this comment.
utility function should be added to /utils and covered by tests
| interface LongTaskAttribution { | ||
| name: string; | ||
| entryType: string; | ||
| containerType?: string; | ||
| containerSrc?: string; | ||
| containerId?: string; | ||
| containerName?: string; | ||
| } |
There was a problem hiding this comment.
move types to the /types folder or the @hawk.so/types package.
and add clear jsdocs to each property.
| */ | ||
| function observeLongTasks(onEntry: (e: LongTaskEvent) => void): void { | ||
| if (!supportsEntryType('longtask')) { | ||
| log('Long Tasks API is not supported in this browser', 'info'); |
There was a problem hiding this comment.
Such logs should not be shown by default, only when user explicitly enables such logs
| const relevantScripts = loaf.scripts?.filter((s) => s.sourceURL || s.sourceFunctionName); | ||
|
|
||
| const scripts = relevantScripts?.length | ||
| ? relevantScripts.reduce<Json>((acc, s, i) => { |
| * | ||
| * @default enabled with default options (both longTasks and longAnimationFrames on) | ||
| */ | ||
| mainThreadBlocking?: false | MainThreadBlockingOptions; |
There was a problem hiding this comment.
why Long Animation Frames is configured via mainThreadBlocking option. It's confusing.
Maybe we can use a better naming
…ThreadBlocking for clarity
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Peter <specc.dev@gmail.com>
Co-authored-by: Peter <specc.dev@gmail.com>
…interfaces and improving script serialization
- Introduced `web-vitals` as a peer dependency and added it to the package.json. - Updated README to reflect new issues detection configuration, including `issues.webVitals`. - Refactored the catcher to utilize an `IssuesMonitor` for handling global errors and performance issues. - Removed the Long Tasks and LoAF tracking functionality, consolidating it under the new issues detection system.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (10)
packages/javascript/src/addons/issues.ts:298
- JSDoc comment is missing a description. It should describe what this function does, e.g., "Resolve and validate the threshold value, applying minimum and default values as needed."
/**
*
* @param value custom threshold
* @param fallback default threshold
*/
packages/javascript/src/addons/issues.ts:319
- JSDoc comment is missing a description. It should describe what this function does, e.g., "Serialize a LoAF script entry to a compact JSON object, omitting null and undefined values."
/**
*
* @param script loaf script entry
*/
packages/javascript/src/addons/issues.ts:338
- JSDoc comment is missing a description. It should describe what this function does, e.g., "Serialize a Web Vitals report to a compact JSON object for transmission."
/**
*
* @param report aggregated vitals report
*/
packages/javascript/src/addons/issues.ts:81
- JSDoc comment is missing a description. It should describe what this method does, e.g., "Observe Long Tasks using the Performance API and emit issues when duration exceeds threshold."
/**
*
* @param thresholdMs max allowed duration
* @param onIssue issue callback
*/
packages/javascript/src/addons/issues.ts:203
- JSDoc comment is missing a description. It should describe what this method does, e.g., "Initialize Web Vitals monitoring and emit issue when at least one metric is poor."
/**
*
* @param onIssue issue callback
*/
packages/javascript/src/addons/issues.ts:311
- JSDoc comment is missing a description. It should describe what this function does, e.g., "Format a Web Vital metric value for display (fixed precision for CLS, milliseconds for others)."
/**
*
* @param name metric name
* @param value metric value
*/
packages/javascript/src/addons/issues.ts:121
- The observe call should have the options object on a single line or properly formatted. The current formatting with
buffered: trueon a separate line without proper indentation is inconsistent with typical JavaScript style conventions.
this.longTaskObserver.observe({ type: 'longtask',
buffered: true });
packages/javascript/src/addons/issues.ts:194
- The observe call should have the options object on a single line or properly formatted. The current formatting with
buffered: trueon a separate line without proper indentation is inconsistent with typical JavaScript style conventions.
this.loafObserver.observe({ type: 'long-animation-frame',
buffered: true });
packages/javascript/src/addons/issues.ts:131
- JSDoc comment is missing a description. It should describe what this method does, e.g., "Observe Long Animation Frames using the Performance API and emit issues when duration exceeds threshold."
/**
*
* @param thresholdMs max allowed duration
* @param onIssue issue callback
*/
packages/javascript/src/addons/issues.ts:281
- JSDoc comment is missing a description. It should describe what this function does, e.g., "Check if the Performance API supports a specific entry type."
/**
*
* @param type performance entry type
*/
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const tryReport = (): void => { | ||
| if (this.destroyed || reported || collected.length < TOTAL_WEB_VITALS) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The Web Vitals reporting waits for all 5 metrics (LCP, FCP, TTFB, INP, CLS) to be collected before reporting. However, not all metrics may be available on every page (e.g., INP requires user interaction, LCP may not occur on certain pages). This means poor metrics might never be reported if the page doesn't trigger all 5 metrics. Consider reporting when a timeout expires or when the page is about to be unloaded, rather than waiting for all 5 metrics indefinitely.
| export class IssuesMonitor { | ||
| private longTaskObserver: PerformanceObserver | null = null; | ||
| private loafObserver: PerformanceObserver | null = null; | ||
| private destroyed = false; | ||
|
|
||
| /** | ||
| * Initialize selected issue detectors. | ||
| * | ||
| * @param options detectors config | ||
| * @param onIssue issue callback | ||
| */ | ||
| public init(options: IssuesOptions, onIssue: (event: IssueEvent) => void): void { | ||
| if (options.longTasks !== false) { | ||
| this.observeLongTasks( | ||
| resolveThreshold(options.longTasks?.thresholdMs, DEFAULT_LONG_TASK_THRESHOLD_MS), | ||
| onIssue | ||
| ); | ||
| } | ||
|
|
||
| if (options.longAnimationFrames !== false) { | ||
| this.observeLoAF( | ||
| resolveThreshold(options.longAnimationFrames?.thresholdMs, DEFAULT_LOAF_THRESHOLD_MS), | ||
| onIssue | ||
| ); | ||
| } | ||
|
|
||
| if (options.webVitals === true) { | ||
| this.observeWebVitals(onIssue); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Cleanup active observers. | ||
| */ | ||
| public destroy(): void { | ||
| this.destroyed = true; | ||
| this.longTaskObserver?.disconnect(); | ||
| this.loafObserver?.disconnect(); | ||
| this.longTaskObserver = null; | ||
| this.loafObserver = null; | ||
| } | ||
|
|
||
| /** | ||
| * | ||
| * @param thresholdMs max allowed duration | ||
| * @param onIssue issue callback | ||
| */ | ||
| private observeLongTasks(thresholdMs: number, onIssue: (event: IssueEvent) => void): void { | ||
| if (!supportsEntryType('longtask')) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| this.longTaskObserver = new PerformanceObserver((list) => { | ||
| for (const entry of list.getEntries()) { | ||
| if (this.destroyed) { | ||
| return; | ||
| } | ||
|
|
||
| const task = entry as LongTaskPerformanceEntry; | ||
| const durationMs = Math.round(task.duration); | ||
| const attr = task.attribution?.[0]; | ||
|
|
||
| if (durationMs < thresholdMs) { | ||
| continue; | ||
| } | ||
|
|
||
| const details = compactJson([ | ||
| ['kind', 'longtask'], | ||
| ['entryName', task.name], | ||
| ['startTime', Math.round(task.startTime)], | ||
| ['durationMs', durationMs], | ||
| ['containerType', attr?.containerType], | ||
| ['containerSrc', attr?.containerSrc], | ||
| ['containerId', attr?.containerId], | ||
| ['containerName', attr?.containerName], | ||
| ]); | ||
|
|
||
| onIssue({ | ||
| title: `Long Task ${durationMs} ms`, | ||
| context: { freezeDetection: details }, | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| this.longTaskObserver.observe({ type: 'longtask', | ||
| buffered: true }); | ||
| } catch { | ||
| this.longTaskObserver = null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * | ||
| * @param thresholdMs max allowed duration | ||
| * @param onIssue issue callback | ||
| */ | ||
| private observeLoAF(thresholdMs: number, onIssue: (event: IssueEvent) => void): void { | ||
| if (!supportsEntryType('long-animation-frame')) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| this.loafObserver = new PerformanceObserver((list) => { | ||
| for (const entry of list.getEntries()) { | ||
| if (this.destroyed) { | ||
| return; | ||
| } | ||
|
|
||
| const loaf = entry as LoAFEntry; | ||
| const durationMs = Math.round(loaf.duration); | ||
|
|
||
| if (durationMs < thresholdMs) { | ||
| continue; | ||
| } | ||
|
|
||
| const blockingDurationMs = loaf.blockingDuration !== undefined && loaf.blockingDuration !== null | ||
| ? Math.round(loaf.blockingDuration) | ||
| : null; | ||
|
|
||
| const relevantScripts = loaf.scripts?.filter((s) => s.sourceURL || s.sourceFunctionName) ?? []; | ||
| const scripts = relevantScripts.length | ||
| ? relevantScripts.reduce<Json>((acc, script, i) => { | ||
| acc[`script_${i}`] = serializeScript(script); | ||
|
|
||
| return acc; | ||
| }, {}) | ||
| : null; | ||
|
|
||
| const details = compactJson([ | ||
| ['kind', 'loaf'], | ||
| ['startTime', Math.round(loaf.startTime)], | ||
| ['durationMs', durationMs], | ||
| ['blockingDurationMs', blockingDurationMs], | ||
| ['renderStart', loaf.renderStart != null ? Math.round(loaf.renderStart) : null], | ||
| ['styleAndLayoutStart', loaf.styleAndLayoutStart != null ? Math.round(loaf.styleAndLayoutStart) : null], | ||
| ['firstUIEventTimestamp', loaf.firstUIEventTimestamp != null ? Math.round(loaf.firstUIEventTimestamp) : null], | ||
| ['scripts', scripts], | ||
| ]); | ||
|
|
||
| const blockingNote = blockingDurationMs !== null | ||
| ? ` (blocking ${blockingDurationMs} ms)` | ||
| : ''; | ||
|
|
||
| const topScript = relevantScripts[0]; | ||
| const culprit = topScript?.sourceFunctionName | ||
| || topScript?.invoker | ||
| || topScript?.sourceURL | ||
| || ''; | ||
| const culpritNote = culprit ? ` — ${culprit}` : ''; | ||
|
|
||
| onIssue({ | ||
| title: `Long Animation Frame ${durationMs} ms${blockingNote}${culpritNote}`, | ||
| context: { freezeDetection: details }, | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| this.loafObserver.observe({ type: 'long-animation-frame', | ||
| buffered: true }); | ||
| } catch { | ||
| this.loafObserver = null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * | ||
| * @param onIssue issue callback | ||
| */ | ||
| private observeWebVitals(onIssue: (event: IssueEvent) => void): void { | ||
| void import('web-vitals').then(({ onCLS, onINP, onLCP, onFCP, onTTFB }) => { | ||
| const collected: WebVitalMetric[] = []; | ||
| let reported = false; | ||
|
|
||
| const tryReport = (): void => { | ||
| if (this.destroyed || reported || collected.length < TOTAL_WEB_VITALS) { | ||
| return; | ||
| } | ||
|
|
||
| reported = true; | ||
|
|
||
| const poor = collected.filter((metric) => metric.rating === 'poor'); | ||
|
|
||
| if (poor.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| const summary = poor | ||
| .map((metric) => { | ||
| const thresholds = METRIC_THRESHOLDS[metric.name]; | ||
| const threshold = thresholds ? ` (poor > ${formatValue(metric.name, thresholds[1])})` : ''; | ||
|
|
||
| return `${metric.name} = ${formatValue(metric.name, metric.value)}${threshold}`; | ||
| }) | ||
| .join(', '); | ||
|
|
||
| const report: WebVitalsReport = { | ||
| summary, | ||
| poorCount: poor.length, | ||
| metrics: collected.reduce<Record<string, WebVitalMetric>>((acc, metric) => { | ||
| acc[metric.name] = metric; | ||
|
|
||
| return acc; | ||
| }, {}), | ||
| }; | ||
|
|
||
| onIssue({ | ||
| title: `Poor Web Vitals: ${summary}`, | ||
| context: { | ||
| webVitals: serializeWebVitalsReport(report), | ||
| }, | ||
| }); | ||
| }; | ||
|
|
||
| const collect = (metric: { name: string; value: number; rating: WebVitalRating; delta: number }): void => { | ||
| if (this.destroyed || reported) { | ||
| return; | ||
| } | ||
|
|
||
| collected.push({ | ||
| name: metric.name, | ||
| value: metric.value, | ||
| rating: metric.rating, | ||
| delta: metric.delta, | ||
| }); | ||
|
|
||
| tryReport(); | ||
| }; | ||
|
|
||
| onCLS(collect); | ||
| onINP(collect); | ||
| onLCP(collect); | ||
| onFCP(collect); | ||
| onTTFB(collect); | ||
| }).catch(() => { | ||
| log( | ||
| 'web-vitals package is required for Web Vitals tracking. Install it with: npm i web-vitals', | ||
| 'warn' | ||
| ); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * | ||
| * @param type performance entry type | ||
| */ | ||
| function supportsEntryType(type: string): boolean { | ||
| try { | ||
| return ( | ||
| typeof PerformanceObserver !== 'undefined' && | ||
| typeof PerformanceObserver.supportedEntryTypes !== 'undefined' && | ||
| PerformanceObserver.supportedEntryTypes.includes(type) | ||
| ); | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * | ||
| * @param value custom threshold | ||
| * @param fallback default threshold | ||
| */ | ||
| function resolveThreshold(value: number | undefined, fallback: number): number { | ||
| if (typeof value !== 'number' || Number.isNaN(value)) { | ||
| return Math.max(MIN_ISSUE_THRESHOLD_MS, fallback); | ||
| } | ||
|
|
||
| return Math.max(MIN_ISSUE_THRESHOLD_MS, Math.round(value)); | ||
| } | ||
|
|
||
| /** | ||
| * | ||
| * @param name metric name | ||
| * @param value metric value | ||
| */ | ||
| function formatValue(name: string, value: number): string { | ||
| return name === 'CLS' ? value.toFixed(3) : `${Math.round(value)}ms`; | ||
| } | ||
|
|
||
| /** | ||
| * | ||
| * @param script loaf script entry | ||
| */ | ||
| function serializeScript(script: LoAFScript): Json { | ||
| return compactJson([ | ||
| ['invoker', script.invoker], | ||
| ['invokerType', script.invokerType], | ||
| ['sourceURL', script.sourceURL], | ||
| ['sourceFunctionName', script.sourceFunctionName], | ||
| ['sourceCharPosition', script.sourceCharPosition != null && script.sourceCharPosition >= 0 ? script.sourceCharPosition : null], | ||
| ['duration', Math.round(script.duration)], | ||
| ['executionStart', script.executionStart != null ? Math.round(script.executionStart) : null], | ||
| ['forcedStyleAndLayoutDuration', script.forcedStyleAndLayoutDuration != null ? Math.round(script.forcedStyleAndLayoutDuration) : null], | ||
| ['pauseDuration', script.pauseDuration != null ? Math.round(script.pauseDuration) : null], | ||
| ['windowAttribution', script.windowAttribution], | ||
| ]); | ||
| } | ||
|
|
||
| /** | ||
| * | ||
| * @param report aggregated vitals report | ||
| */ | ||
| function serializeWebVitalsReport(report: WebVitalsReport): Json { | ||
| const metrics = Object.entries(report.metrics).reduce<Json>((acc, [name, metric]) => { | ||
| acc[name] = compactJson([ | ||
| ['name', metric.name], | ||
| ['value', metric.value], | ||
| ['rating', metric.rating], | ||
| ['delta', metric.delta], | ||
| ]); | ||
|
|
||
| return acc; | ||
| }, {}); | ||
|
|
||
| return compactJson([ | ||
| ['summary', report.summary], | ||
| ['poorCount', report.poorCount], | ||
| ['metrics', metrics], | ||
| ]); | ||
| } |
There was a problem hiding this comment.
The IssuesMonitor class lacks test coverage. The codebase has comprehensive test coverage for similar addons (e.g., BreadcrumbManager in breadcrumbs.test.ts). Critical behaviors that should be tested include: threshold validation and clamping, observer initialization and cleanup, metric collection logic, and the aggregation behavior for Web Vitals. Tests should also verify error handling when APIs are unsupported or when web-vitals package is missing.
packages/javascript/src/catcher.ts
Outdated
|
|
||
| if (shouldDetectPerformanceIssues) { | ||
| this.issuesMonitor.init(issues, (entry) => this.send(entry.title, entry.context)); | ||
| } |
There was a problem hiding this comment.
The performance issues monitor is only initialized when errors are enabled. This means if a user sets issues.errors: false but issues.longTasks: true or issues.webVitals: true, those features won't work. Performance monitoring should be independent of error handling and should initialize regardless of the errors setting.
| if (shouldDetectPerformanceIssues) { | |
| this.issuesMonitor.init(issues, (entry) => this.send(entry.title, entry.context)); | |
| } | |
| } | |
| if (shouldDetectPerformanceIssues) { | |
| this.issuesMonitor.init(issues, (entry) => this.send(entry.title, entry.context)); |
| public init(options: IssuesOptions, onIssue: (event: IssueEvent) => void): void { | ||
| if (options.longTasks !== false) { | ||
| this.observeLongTasks( | ||
| resolveThreshold(options.longTasks?.thresholdMs, DEFAULT_LONG_TASK_THRESHOLD_MS), | ||
| onIssue | ||
| ); | ||
| } | ||
|
|
||
| if (options.longAnimationFrames !== false) { | ||
| this.observeLoAF( | ||
| resolveThreshold(options.longAnimationFrames?.thresholdMs, DEFAULT_LOAF_THRESHOLD_MS), | ||
| onIssue | ||
| ); | ||
| } | ||
|
|
||
| if (options.webVitals === true) { | ||
| this.observeWebVitals(onIssue); | ||
| } | ||
| } |
There was a problem hiding this comment.
The init method can be called multiple times, which would create duplicate PerformanceObserver instances and result in duplicate issue reports. Consider adding a guard to prevent re-initialization, similar to how BreadcrumbManager has an isInitialized flag that prevents duplicate initialization.
packages/javascript/README.md
Outdated
| - **Long Tasks API** — reports any task taking longer than 50 ms. | ||
| - **Long Animation Frames (LoAF)** — reports frames taking longer than 50 ms with richer script attribution (Chrome 123+, Edge 123+). | ||
|
|
||
| Both freeze detectors are enabled by default. If one API is unsupported, the other still works. | ||
| Each detected freeze is reported immediately with detailed context (duration, blocking time, scripts involved, etc.). | ||
| `thresholdMs` is the max allowed duration budget. Hawk emits an issue when measured duration is equal to or greater than this value. Values below `50ms` are clamped to `50ms`. |
There was a problem hiding this comment.
The documentation states that Long Tasks API "reports any task taking longer than 50 ms" but this is the browser's inherent threshold, not configurable by the code. The actual behavior is: the browser reports tasks >= 50ms, then the code filters these based on the configured thresholdMs (default 100ms). The documentation should clarify that 50ms is the browser's minimum threshold, and the code applies an additional filter on top of that.
| - **Long Tasks API** — reports any task taking longer than 50 ms. | |
| - **Long Animation Frames (LoAF)** — reports frames taking longer than 50 ms with richer script attribution (Chrome 123+, Edge 123+). | |
| Both freeze detectors are enabled by default. If one API is unsupported, the other still works. | |
| Each detected freeze is reported immediately with detailed context (duration, blocking time, scripts involved, etc.). | |
| `thresholdMs` is the max allowed duration budget. Hawk emits an issue when measured duration is equal to or greater than this value. Values below `50ms` are clamped to `50ms`. | |
| - **Long Tasks API** — the browser reports tasks that block the main thread for at least 50 ms (this 50 ms minimum is defined by the browser and cannot be lowered by Hawk). | |
| - **Long Animation Frames (LoAF)** — the browser reports frames taking at least 50 ms with richer script attribution (Chrome 123+, Edge 123+); this 50 ms minimum is also browser-defined. | |
| Both freeze detectors are enabled by default. If one API is unsupported, the other still works. | |
| Each detected freeze is reported immediately with detailed context (duration, blocking time, scripts involved, etc.). | |
| `thresholdMs` is the max allowed duration budget applied on top of what the browser reports. Hawk emits an issue only when the measured duration is equal to or greater than `thresholdMs`. Because the browser never reports events under 50 ms, values below `50ms` are clamped to `50ms`. |
Dobrunia
changed the title
Summary
Defaults
Dependency / Bundle behavior
Browser support