Skip to content

Commit 1e8fdd1

Browse files
authored
feat: Add serializeAttribute option to breadcrumbs / dom. (getsentry#3620)
* Add `serializeAttribute` option to breadcrumbs / dom. * Add jsdom as a dev-dependency of utils package. * Enable `jsdom` legacy versions for `utils` package.
1 parent 85bb9f6 commit 1e8fdd1

File tree

5 files changed

+76
-15
lines changed

5 files changed

+76
-15
lines changed

packages/browser/src/integrations/breadcrumbs.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
/** JSDoc */
1515
interface BreadcrumbsOptions {
1616
console: boolean;
17-
dom: boolean;
17+
dom: boolean | { serializeAttribute: string };
1818
fetch: boolean;
1919
history: boolean;
2020
sentry: boolean;
@@ -162,12 +162,13 @@ export class Breadcrumbs implements Integration {
162162
// eslint-disable-next-line @typescript-eslint/no-explicit-any
163163
private _domBreadcrumb(handlerData: { [key: string]: any }): void {
164164
let target;
165+
const keyAttr = typeof this._options.dom === 'object' ? this._options.dom.serializeAttribute : undefined;
165166

166167
// Accessing event.target can throw (see getsentry/raven-js#838, #768)
167168
try {
168169
target = handlerData.event.target
169-
? htmlTreeAsString(handlerData.event.target as Node)
170-
: htmlTreeAsString((handlerData.event as unknown) as Node);
170+
? htmlTreeAsString(handlerData.event.target as Node, keyAttr)
171+
: htmlTreeAsString((handlerData.event as unknown) as Node, keyAttr);
171172
} catch (e) {
172173
target = '<unknown>';
173174
}

packages/utils/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
"@sentry-internal/eslint-config-sdk": "6.5.1",
2424
"chai": "^4.1.2",
2525
"jest": "^24.7.1",
26+
"jsdom": "^16.2.2",
2627
"npm-run-all": "^4.1.2",
2728
"prettier": "1.19.0",
2829
"rimraf": "^2.6.3",

packages/utils/src/browser.ts

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { isString } from './is';
66
* e.g. [HTMLElement] => body > div > input#foo.btn[name=baz]
77
* @returns generated DOM path
88
*/
9-
export function htmlTreeAsString(elem: unknown): string {
9+
export function htmlTreeAsString(elem: unknown, keyAttr?: string): string {
1010
type SimpleNode = {
1111
parentNode: SimpleNode;
1212
} | null;
@@ -28,7 +28,7 @@ export function htmlTreeAsString(elem: unknown): string {
2828

2929
// eslint-disable-next-line no-plusplus
3030
while (currentElem && height++ < MAX_TRAVERSE_HEIGHT) {
31-
nextStr = _htmlElementAsString(currentElem);
31+
nextStr = _htmlElementAsString(currentElem, keyAttr);
3232
// bail out if
3333
// - nextStr is the 'html' element
3434
// - the length of the string that would be created exceeds MAX_OUTPUT_LEN
@@ -54,7 +54,7 @@ export function htmlTreeAsString(elem: unknown): string {
5454
* e.g. [HTMLElement] => input#foo.btn[name=baz]
5555
* @returns generated DOM path
5656
*/
57-
function _htmlElementAsString(el: unknown): string {
57+
function _htmlElementAsString(el: unknown, keyAttr?: string): string {
5858
const elem = el as {
5959
tagName?: string;
6060
id?: string;
@@ -74,16 +74,22 @@ function _htmlElementAsString(el: unknown): string {
7474
}
7575

7676
out.push(elem.tagName.toLowerCase());
77-
if (elem.id) {
78-
out.push(`#${elem.id}`);
79-
}
8077

81-
// eslint-disable-next-line prefer-const
82-
className = elem.className;
83-
if (className && isString(className)) {
84-
classes = className.split(/\s+/);
85-
for (i = 0; i < classes.length; i++) {
86-
out.push(`.${classes[i]}`);
78+
const keyAttrValue = keyAttr ? elem.getAttribute(keyAttr) : null;
79+
if (keyAttrValue) {
80+
out.push(`[${keyAttr}="${keyAttrValue}"]`);
81+
} else {
82+
if (elem.id) {
83+
out.push(`#${elem.id}`);
84+
}
85+
86+
// eslint-disable-next-line prefer-const
87+
className = elem.className;
88+
if (className && isString(className)) {
89+
classes = className.split(/\s+/);
90+
for (i = 0; i < classes.length; i++) {
91+
out.push(`.${classes[i]}`);
92+
}
8793
}
8894
}
8995
const allowedAttrs = ['type', 'name', 'title', 'alt'];

packages/utils/test/browser.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { JSDOM } from 'jsdom';
2+
3+
import { htmlTreeAsString } from '../src/browser';
4+
5+
beforeAll(() => {
6+
const dom = new JSDOM();
7+
// @ts-ignore need to override global document
8+
global.document = dom.window.document;
9+
});
10+
11+
describe('htmlTreeAsString', () => {
12+
it('generates html tree for a simple element', () => {
13+
const el = document.createElement('ul');
14+
el.innerHTML = `<li class="container">
15+
<button id="err-btn" class="button" />
16+
</li>`;
17+
document.body.appendChild(el);
18+
19+
expect(htmlTreeAsString(document.getElementById('err-btn'))).toBe(
20+
'body > ul > li.container > button#err-btn.button',
21+
);
22+
});
23+
24+
it('inserts pre-defined attribute values by default', () => {
25+
const el = document.createElement('ul');
26+
el.innerHTML = `<li title="container-title" class="container">
27+
<img id="cat" test-id="cat-test-id" alt="kitten" />
28+
</li>`;
29+
document.body.appendChild(el);
30+
31+
expect(htmlTreeAsString(document.getElementById('cat'))).toBe(
32+
'body > ul > li.container[title="container-title"] > img#cat[alt="kitten"]',
33+
);
34+
});
35+
36+
it('insert key attribute instead of class names or ids when serializeAttribute is defined and the element has it', () => {
37+
const el = document.createElement('ul');
38+
el.innerHTML = `<li class="li-class" title="li-title">
39+
<img id="cat-2" class="catimg" test-id="cat-2-test-id"/>
40+
</li>`;
41+
document.body.appendChild(el);
42+
43+
expect(htmlTreeAsString(document.getElementById('cat-2'), 'test-id')).toBe(
44+
'body > ul > li.li-class[title="li-title"] > img[test-id="cat-2-test-id"]',
45+
);
46+
});
47+
});

scripts/test.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ if [[ "$(cut -d. -f1 <<<"$NODE_VERSION")" -le 6 ]]; then
1212
cd packages/tracing
1313
yarn add --dev --ignore-engines --ignore-scripts [email protected]
1414
cd ../..
15+
cd packages/utils
16+
yarn add --dev --ignore-engines --ignore-scripts [email protected]
17+
cd ../..
1518

1619
# only test against @sentry/node and its dependencies - node 6 is too old for anything else to work
1720
yarn test --scope="@sentry/core" --scope="@sentry/hub" --scope="@sentry/minimal" --scope="@sentry/node" --scope="@sentry/utils" --scope="@sentry/tracing"
@@ -23,6 +26,9 @@ elif [[ "$(cut -d. -f1 <<<"$NODE_VERSION")" -le 8 ]]; then
2326
cd packages/tracing
2427
yarn add --dev --ignore-engines --ignore-scripts [email protected]
2528
cd ../..
29+
cd packages/utils
30+
yarn add --dev --ignore-engines --ignore-scripts [email protected]
31+
cd ../..
2632

2733
# ember tests happen separately, and the rest fail on node 8 for various syntax or dependency reasons
2834
yarn test --ignore="@sentry/ember" --ignore="@sentry-internal/eslint-plugin-sdk" --ignore="@sentry/react" --ignore="@sentry/wasm" --ignore="@sentry/gatsby" --ignore="@sentry/serverless" --ignore="@sentry/nextjs"

0 commit comments

Comments
 (0)