-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add breadcrumb management to Catcher class #147
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 adds breadcrumb management functionality to the JavaScript Catcher, enabling automatic and manual tracking of events leading up to errors. Breadcrumbs provide a chronological trail of user actions, network requests, and navigation events that help debug issues by providing context.
Changes:
- Added automatic breadcrumb tracking for fetch/XHR requests, navigation events (History API), and optional UI clicks
- Introduced public API methods:
addBreadcrumb(),getBreadcrumbs(),clearBreadcrumbs() - Updated package to use @hawk.so/types v0.1.38 which includes Breadcrumb type definitions
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updated @hawk.so/types dependency from 0.1.36 to 0.1.38 to support breadcrumb types |
| packages/javascript/package.json | Updated @hawk.so/types dependency and added lint script |
| packages/javascript/src/types/hawk-initial-settings.ts | Added configuration options for breadcrumb management (maxBreadcrumbs, trackFetch, trackNavigation, trackClicks, beforeBreadcrumb hook) |
| packages/javascript/src/types/event.ts | Added breadcrumbs field to HawkJavaScriptEvent type |
| packages/javascript/src/catcher.ts | Integrated BreadcrumbManager singleton, added public API methods, and included breadcrumbs in error events |
| packages/javascript/src/addons/breadcrumbs.ts | New module implementing singleton BreadcrumbManager with automatic tracking and manual API |
| packages/javascript/example/sample-errors.js | Added comprehensive examples demonstrating all breadcrumb types and API usage |
| packages/javascript/example/index.html | Added UI controls for testing breadcrumb functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…anager with popstate event handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 13 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…s to align with new structure
… messages in sample-errors.js
|
|
||
| if (element.id) { | ||
| selector += `#${element.id}`; | ||
| } else if (element.className && typeof element.className === 'string') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems redundant: type of Element.className is always string.
package.json
Outdated
| "url": "git+https://github.com/codex-team/hawk.javascript.git" | ||
| }, | ||
| "devDependencies": { | ||
| "@hawk.so/types": "0.5.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it needed throughout the project? It seems that this dependency is only needed in packages/javascript.
.github/workflows/main.yml
Outdated
| uses: actions/setup-node@v1 | ||
| with: | ||
| node-version: 20.x | ||
| node-version: 24.x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also upgrade node version in .nvmrc so all dev's will have to install correct node version?
Plus we could do something like this:
node-version-file: '.nvmrc'
packages/javascript/README.md
Outdated
| maxValueLength: 512, // Max string length (default: 1024) | ||
| trackFetch: true, // Track fetch/XHR requests (default: true) | ||
| trackNavigation: true, // Track navigation events (default: true) | ||
| trackClicks: false, // Track UI clicks (default: false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why clicks are not ebaled by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it produces a lot of breadcrumbs that are not very informative
packages/javascript/src/catcher.ts
Outdated
| /** | ||
| * Breadcrumbs API interface | ||
| */ | ||
| interface BreadcrumbsAPI { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to types folder
| } else if (element.className && typeof element.className === 'string') { | ||
| selector += `.${element.className.split(' ').filter(Boolean) | ||
| .join('.')}`; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if element has no class and id (for example, just button), you can try to call buildElementSelector() on its parent (recursively) to build a selector like
.some-parent buttonor
.some-grand-parent div button| */ | ||
| public init(options: BreadcrumbsOptions = {}): void { | ||
| if (this.isInitialized) { | ||
| console.warn('[BreadcrumbManager] init has already been called; breadcrumb configuration is global and subsequent init options are ignored.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use utils/log for all logs of Hawk catcher.
| * Add a breadcrumb to the buffer | ||
| * | ||
| * @param breadcrumb - The breadcrumb data to add | ||
| * @param hint - Optional hint object with original event data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
describe what is "hint".
| for (const key in sanitized) { | ||
| if (typeof sanitized[key] === 'string') { | ||
| sanitized[key] = this.trimString(sanitized[key] as string, this.options.maxValueLength); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sanitizer already trims strings
| manager.addBreadcrumb({ | ||
| type: 'request', | ||
| category: 'fetch', | ||
| message: `${method} ${url} failed`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| message: `${method} ${url} failed`, | |
| message: `[FAIL] ${method} ${url}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you move breadcrumbs same errors code to a separate file? current sample-errors.js is already bloated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
…use .nvmrc; reintroduce @hawk.so/types in package.json
Add Breadcrumbs Support to JavaScript Catcher
Adds automatic and manual breadcrumb tracking.
Features:
hawk.breadcrumbs.add(),hawk.breadcrumbs.get(),hawk.breadcrumbs.clear()beforeBreadcrumbhook