-
Notifications
You must be signed in to change notification settings - Fork 146
fix(rum-angular): capture angular navigation errors #1662
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
Conversation
🤖 GitHub commentsJust comment with:
|
🔍 Preview links for changed docs |
vigneshshanmugam
left a comment
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.
LGTM, added few nits - but no blocking comments.
packages/rum/src/index.d.ts
Outdated
| } | ||
| } | ||
| type Payload = { | ||
| transactions: TransactionPayload[] |
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.
nit: I forgot if we explicitly exclude the fields from Transaction, can we do Partial and omit few keys?
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.
okay, I'll leave id, type and name
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.
commit abc965d
I've made also Span and Error partial
package.json
Outdated
| "engines": { | ||
| "node": ">=12 <=16", | ||
| "npm": "6" | ||
| "node": "^20.19.0 || ^22.12.0 || >=24.0.0" |
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 not >= 20? as nvmrc uses 20 ?
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.
commit abc965d
The Angular integration will now capture navigation errors
Fixes: #1660
This PR also add a small extra check for payloads that have been modified by
addFilterAPI. For that we updated the type definitions of the filter function so users will have more assistance in their IDEsFixes: #1658
Closes: #1659