Skip to content

ref(browser): Improve type safety of breadcrumbs integration #7382

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

Merged
merged 1 commit into from
Mar 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 21 additions & 20 deletions packages/browser/src/integrations/breadcrumbs.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
/* eslint-disable max-lines */
import { getCurrentHub } from '@sentry/core';
import type { Event, Integration } from '@sentry/types';
import type { Event as SentryEvent, HandlerDataFetch, Integration, SentryWrappedXMLHttpRequest } from '@sentry/types';
import {
addInstrumentationHandler,
getEventDescription,
Expand All @@ -14,6 +14,8 @@ import {

import { WINDOW } from '../helpers';

type HandlerData = Record<string, unknown>;

/** JSDoc */
interface BreadcrumbsOptions {
console: boolean;
Expand Down Expand Up @@ -99,7 +101,7 @@ export class Breadcrumbs implements Integration {
/**
* Adds a breadcrumb for Sentry events or transactions if this option is enabled.
*/
public addSentryBreadcrumb(event: Event): void {
public addSentryBreadcrumb(event: SentryEvent): void {
if (this.options.sentry) {
getCurrentHub().addBreadcrumb(
{
Expand All @@ -120,10 +122,8 @@ export class Breadcrumbs implements Integration {
* A HOC that creaes a function that creates breadcrumbs from DOM API calls.
* This is a HOC so that we get access to dom options in the closure.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function _domBreadcrumb(dom: BreadcrumbsOptions['dom']): (handlerData: { [key: string]: any }) => void {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function _innerDomBreadcrumb(handlerData: { [key: string]: any }): void {
function _domBreadcrumb(dom: BreadcrumbsOptions['dom']): (handlerData: HandlerData) => void {
function _innerDomBreadcrumb(handlerData: HandlerData): void {
let target;
let keyAttrs = typeof dom === 'object' ? dom.serializeAttribute : undefined;

Expand All @@ -143,9 +143,10 @@ function _domBreadcrumb(dom: BreadcrumbsOptions['dom']): (handlerData: { [key: s

// Accessing event.target can throw (see getsentry/raven-js#838, #768)
try {
target = handlerData.event.target
? htmlTreeAsString(handlerData.event.target as Node, { keyAttrs, maxStringLength })
: htmlTreeAsString(handlerData.event as unknown as Node, { keyAttrs, maxStringLength });
const event = handlerData.event as Event | Node;
target = _isEvent(event)
? htmlTreeAsString(event.target, { keyAttrs, maxStringLength })
: htmlTreeAsString(event, { keyAttrs, maxStringLength });
} catch (e) {
target = '<unknown>';
}
Expand Down Expand Up @@ -173,8 +174,7 @@ function _domBreadcrumb(dom: BreadcrumbsOptions['dom']): (handlerData: { [key: s
/**
* Creates breadcrumbs from console API calls
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function _consoleBreadcrumb(handlerData: { [key: string]: any }): void {
function _consoleBreadcrumb(handlerData: HandlerData & { args: unknown[]; level: string }): void {
// This is a hack to fix a Vue3-specific bug that causes an infinite loop of
// console warnings. This happens when a Vue template is rendered with
// an undeclared variable, which we try to stringify, ultimately causing
Expand Down Expand Up @@ -216,8 +216,7 @@ function _consoleBreadcrumb(handlerData: { [key: string]: any }): void {
/**
* Creates breadcrumbs from XHR API calls
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function _xhrBreadcrumb(handlerData: { [key: string]: any }): void {
function _xhrBreadcrumb(handlerData: HandlerData & { xhr: SentryWrappedXMLHttpRequest }): void {
if (handlerData.endTimestamp) {
// We only capture complete, non-sentry requests
if (handlerData.xhr.__sentry_own_request__) {
Expand Down Expand Up @@ -249,8 +248,7 @@ function _xhrBreadcrumb(handlerData: { [key: string]: any }): void {
/**
* Creates breadcrumbs from fetch API calls
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function _fetchBreadcrumb(handlerData: { [key: string]: any }): void {
function _fetchBreadcrumb(handlerData: HandlerData & HandlerDataFetch): void {
// We only capture complete fetch requests
if (!handlerData.endTimestamp) {
return;
Expand Down Expand Up @@ -280,7 +278,7 @@ function _fetchBreadcrumb(handlerData: { [key: string]: any }): void {
category: 'fetch',
data: {
...handlerData.fetchData,
status_code: handlerData.response.status,
status_code: handlerData.response && handlerData.response.status,
},
type: 'http',
},
Expand All @@ -295,10 +293,9 @@ function _fetchBreadcrumb(handlerData: { [key: string]: any }): void {
/**
* Creates breadcrumbs from history API calls
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function _historyBreadcrumb(handlerData: { [key: string]: any }): void {
let from = handlerData.from;
let to = handlerData.to;
function _historyBreadcrumb(handlerData: HandlerData & { from: string; to: string }): void {
let from: string | undefined = handlerData.from;
let to: string | undefined = handlerData.to;
const parsedLoc = parseUrl(WINDOW.location.href);
let parsedFrom = parseUrl(from);
const parsedTo = parseUrl(to);
Expand All @@ -325,3 +322,7 @@ function _historyBreadcrumb(handlerData: { [key: string]: any }): void {
},
});
}

function _isEvent(event: unknown): event is Event {
return event && !!(event as Record<string, unknown>).target;
}
1 change: 1 addition & 0 deletions packages/types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,5 +94,6 @@ export type {
export type { User, UserFeedback } from './user';
export type { WrappedFunction } from './wrappedfunction';
export type { Instrumenter } from './instrumenter';
export type { HandlerDataFetch, SentryWrappedXMLHttpRequest } from './instrument';

export type { BrowserClientReplayOptions } from './browseroptions';
23 changes: 23 additions & 0 deletions packages/types/src/instrument.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
type XHRSendInput = null | Blob | BufferSource | FormData | URLSearchParams | string;

export interface SentryWrappedXMLHttpRequest extends XMLHttpRequest {
[key: string]: any;
__sentry_xhr__?: {
method?: string;
url?: string;
status_code?: number;
body?: XHRSendInput;
};
}

interface SentryFetchData {
method: string;
url: string;
}

export interface HandlerDataFetch {
args: any[];
fetchData: SentryFetchData;
startTimestamp: number;
response?: Response;
}
17 changes: 2 additions & 15 deletions packages/utils/src/instrument.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable max-lines */
/* eslint-disable @typescript-eslint/no-explicit-any */
/* eslint-disable @typescript-eslint/ban-types */
import type { WrappedFunction } from '@sentry/types';
import type { HandlerDataFetch, SentryWrappedXMLHttpRequest, WrappedFunction } from '@sentry/types';

import { isInstanceOf, isString } from './is';
import { CONSOLE_LEVELS, logger } from './logger';
Expand Down Expand Up @@ -136,7 +136,7 @@ function instrumentFetch(): void {

fill(WINDOW, 'fetch', function (originalFetch: () => void): () => void {
return function (...args: any[]): void {
const handlerData = {
const handlerData: HandlerDataFetch = {
args,
fetchData: {
method: getFetchMethod(args),
Expand Down Expand Up @@ -175,19 +175,6 @@ function instrumentFetch(): void {
});
}

type XHRSendInput = null | Blob | BufferSource | FormData | URLSearchParams | string;

/** JSDoc */
interface SentryWrappedXMLHttpRequest extends XMLHttpRequest {
[key: string]: any;
__sentry_xhr__?: {
method?: string;
url?: string;
status_code?: number;
body?: XHRSendInput;
};
}

/* eslint-disable @typescript-eslint/no-unsafe-member-access */
/** Extract `method` from fetch call arguments */
function getFetchMethod(fetchArgs: any[] = []): string {
Expand Down