8000 misc: Go through some TODOs and fix/remove them · phthhieu/sentry-javascript@da6767f · GitHub
[go: up one dir, main page]
Skip to content

Commit da6767f

Browse files
kamilogorekkamilogorek
committed
misc: Go through some TODOs and fix/remove them
1 parent b887fb1 commit da6767f

File tree

10 files changed

+17
-31
lines changed

10 files changed

+17
-31
lines changed

‎packages/browser/src/integrations/breadcrumbs.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,6 @@ export class Breadcrumbs implements Integration {
131131
url = String(fetchInput);
132132
}
133133

134-
// TODO: Filter our own requests
135-
// if Sentry key appears in URL, don't capture, as it's our own request
136-
// if (url.indexOf(self._globalKey) !== -1) {
137-
// return originalFetch.apply(this, args);
138-
// }
139-
140134
if (args[1] && args[1].method) {
141135
method = args[1].method;
142136
}
@@ -269,10 +263,6 @@ export class Breadcrumbs implements Integration {
269263
'open',
270264
originalOpen =>
271265
function(this: SentryWrappedXMLHttpRequest, ...args: any[]): void {
272-
// TODO: Filter our own requests
273-
// preserve arity
274-
// if Sentry key appears in URL, don't capture
275-
// && url.indexOf(self._globalKey) === -1)
276266
if (isString(args[1])) {
277267
this.__sentry_xhr__ = {
278268
method: args[0],

‎packages/browser/src/integrations/helpers.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ let keypressTimeout: number | undefined;
88
let lastCapturedEvent: Event | undefined;
99
let ignoreOnError: number = -1;
1010

11-
// TODO: Just temporary build fix for unused variable
11+
// TODO: Fix `ignoreNextOnError`. Just temporary build fix for unused variable
1212
ignoreOnError = ignoreOnError + 1;
1313

1414
/** JSDoc */
@@ -62,7 +62,6 @@ export function wrap(
6262
// expected behavior and NOT indicative of a bug with Raven.js.
6363
return fn.apply(undefined, args);
6464
} catch (ex) {
65-
// TODO: Fix `ignoreNextOnError`
6665
ignoreNextOnError();
6766

6867
getDefaultHub().withScope(async () => {

‎packages/browser/src/parsers.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ export function prepareFramesForEvent(stack: TraceKitStackFrame[]): StackFrame[]
6060
let localStack = stack;
6161
const firstFrameFunction = localStack[0].func || '';
6262

63-
// TODO: This could be smarter
6463
if (firstFrameFunction.includes('captureMessage') || firstFrameFunction.includes('captureException')) {
6564
localStack = localStack.slice(1);
6665
}

‎packages/browser/test/integrations/inboundfilters.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ describe('InboundFilters', () => {
5050
});
5151
});
5252

53-
describe('ignoreErrors', async () => {
53+
describe('ignore 67E6 Errors', () => {
5454
const messageEvent = {
5555
message: 'captureMessage',
5656
};

‎packages/core/src/base.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -292,10 +292,6 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
292292
};
293293
}
294294

295-
// TODO: Add breadcrumb with our own event?
296-
// Or should it be handled by the xhr/fetch integration itself?
297-
// Or maybe some other integration that'd use `afterSend`?
298-
299295
const finalEvent = beforeSend ? beforeSend(prepared) : prepared;
300296
const response = await send(finalEvent);
301297

‎packages/node/src/handlers.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { logger } from '@sentry/core';
12
import { getHubFromCarrier } from '@sentry/hub';
23
import { SentryEvent, Severity } from '@sentry/types';
34
import { serialize } from '@sentry/utils/object';
@@ -125,9 +126,6 @@ function parseRequest(
125126
node: global.process.version,
126127
},
127128
modules: getModules(),
128-
// TODO: `platform` shouldn't be relying on `parseRequest` usage
129-
// or we could just change the name and make it generic middleware
130-
platform: 'node',
131129
request: {
132130
...event.request,
133131
...extractRequestData(req),
@@ -148,7 +146,6 @@ function parseRequest(
148146
/** JSDoc */
149147
export function requestHandler(): (req: Request, res: Response, next: () => void) => void {
150148
return function sentryRequestMiddleware(req: Request, _res: Response, next: () => void): void {
151-
// TODO: Do we even need domain when we use middleware like approach? — Kamil
152149
const local = domain.create();
153150
const hub = getHubFromCarrier(req);
154151
hub.bindClient(getDefaultHub().getClient());
@@ -237,8 +234,7 @@ export function makeErrorHandler(
237234
});
238235
} else if (calledFatalError) {
239236
// we hit an error *after* calling onFatalError - pretty boned at this point, just shut it down
240-
// TODO: Use consoleAlert or some other way to log our debug messages
241-
console.warn('uncaught exception after calling fatal error shutdown callback - this is bad! forcing shutdown');
237+
logger.warn('uncaught exception after calling fatal error shutdown callback - this is bad! forcing shutdown');
242238
defaultOnFatalError(error);
243239
} else if (!caughtSecondError) {
244240
// two cases for how we can hit this branch:

‎packages/node/src/integrations/clientoptions.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,17 @@ export class ClientOptions implements Integration {
1313
* @inheritDoc
1414
*/
1515
public install(options: NodeOptions = {}): void {
16-
if (options.serverName) {
17-
getDefaultHub().addEventProcessor(async (event: SentryEvent) => ({
16+
getDefaultHub().addEventProcessor(async (event: SentryEvent) => {
17+
const preparedEvent: SentryEvent = {
1818
...event,
19-
server_name: options.serverName,
20-
}));
21-
}
19+
platform: 'node',
20+
};
21+
22+
if (options.serverName) {
23+
event.server_name = options.serverName;
24+
}
25+
26+
return preparedEvent;
27+
});
2228
}
2329
}

‎packages/node/src/integrations/http.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ function loadWrapper(nativeModule: any): any {
6060
): any {
6161
// Note: this won't capture a breadcrumb if a response never comes
6262
// It would be useful to know if that was the case, though, so
63-
// todo: revisit to see if we can capture sth indicating response never came
63+
// TODO: revisit to see if we can capture sth indicating response never came
6464
// possibility: capture one breadcrumb for "req sent" and one for "res recvd"
6565
// seems excessive but solves the problem and *is* strictly more information
6666
// could be useful for weird response sequencing bug scenarios

‎packages/node/src/parsers.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,6 @@ export function prepareFramesForEvent(stack: StackFrame[]): StackFrame[] {
211211
let localStack = stack;
212212
const firstFrameFunction = localStack[0].function || '';
213213

214-
// TODO: This could be smarter
215214
if (firstFrameFunction.includes('captureMessage') || firstFrameFunction.includes('captureException')) {
216215
localStack = localStack.slice(1);
217216
}

‎packages/utils/src/object.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ export function urlEncode(object: { [key: string]: any }): string {
157157
// Default Node.js REPL depth
158158
const MAX_SERIALIZE_EXCEPTION_DEPTH = 3;
159159
// TODO: Or is it 200kb? 🤔 — Kamil
160+
// NOTE: Yes, it is
160161
// 50kB, as 100kB is max payload size, so half sounds reasonable
161162
const MAX_SERIALIZE_EXCEPTION_SIZE = 50 * 1024;
162163
const MAX_SERIALIZE_KEYS_LENGTH = 40;

0 commit comments

Comments
 (0)
0