From 0086c7468cc0ead7e55e060ccd68a412b9717c88 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 4 Nov 2021 14:10:22 -0700 Subject: [PATCH 1/2] use type predicates in `is` functions --- packages/utils/src/is.ts | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/utils/src/is.ts b/packages/utils/src/is.ts index 08b90c1cf748..755a429d2b6b 100644 --- a/packages/utils/src/is.ts +++ b/packages/utils/src/is.ts @@ -9,7 +9,7 @@ import { Primitive } from '@sentry/types'; * @param wat A value to be checked. * @returns A boolean representing the result. */ -export function isError(wat: any): boolean { +export function isError(wat: any): wat is Error { switch (Object.prototype.toString.call(wat)) { case '[object Error]': return true; @@ -29,7 +29,7 @@ export function isError(wat: any): boolean { * @param wat A value to be checked. * @returns A boolean representing the result. */ -export function isErrorEvent(wat: any): boolean { +export function isErrorEvent(wat: any): wat is ErrorEvent { return Object.prototype.toString.call(wat) === '[object ErrorEvent]'; } @@ -40,7 +40,7 @@ export function isErrorEvent(wat: any): boolean { * @param wat A value to be checked. * @returns A boolean representing the result. */ -export function isDOMError(wat: any): boolean { +export function isDOMError(wat: any): wat is DOMError { return Object.prototype.toString.call(wat) === '[object DOMError]'; } @@ -51,18 +51,18 @@ export function isDOMError(wat: any): boolean { * @param wat A value to be checked. * @returns A boolean representing the result. */ -export function isDOMException(wat: any): boolean { +export function isDOMException(wat: any): wat is DOMException { return Object.prototype.toString.call(wat) === '[object DOMException]'; } /** - * Checks whether given value's type is a string + * Checks whether the given value's type is string * {@link isString}. * * @param wat A value to be checked. * @returns A boolean representing the result. */ -export function isString(wat: any): boolean { +export function isString(wat: any): wat is string { return Object.prototype.toString.call(wat) === '[object String]'; } @@ -84,7 +84,7 @@ export function isPrimitive(wat: any): wat is Primitive { * @param wat A value to be checked. * @returns A boolean representing the result. */ -export function isPlainObject(wat: any): boolean { +export function isPlainObject(wat: any): wat is { [key: string]: unknown } { return Object.prototype.toString.call(wat) === '[object Object]'; } @@ -95,7 +95,7 @@ export function isPlainObject(wat: any): boolean { * @param wat A value to be checked. * @returns A boolean representing the result. */ -export function isEvent(wat: any): boolean { +export function isEvent(wat: any): wat is Event { return typeof Event !== 'undefined' && isInstanceOf(wat, Event); } @@ -106,7 +106,7 @@ export function isEvent(wat: any): boolean { * @param wat A value to be checked. * @returns A boolean representing the result. */ -export function isElement(wat: any): boolean { +export function isElement(wat: any): wat is Element { return typeof Element !== 'undefined' && isInstanceOf(wat, Element); } @@ -117,7 +117,7 @@ export function isElement(wat: any): boolean { * @param wat A value to be checked. * @returns A boolean representing the result. */ -export function isRegExp(wat: any): boolean { +export function isRegExp(wat: any): wat is RegExp { return Object.prototype.toString.call(wat) === '[object RegExp]'; } @@ -140,6 +140,12 @@ export function isThenable(wat: any): boolean { export function isSyntheticEvent(wat: any): boolean { return isPlainObject(wat) && 'nativeEvent' in wat && 'preventDefault' in wat && 'stopPropagation' in wat; } + +// Having the two different types is necessary because some types packages type classes as functions (in addition to +// being objects with a `new` method), and some only type them as `new`-able objects +type Constructor = (...args: any[]) => T; +type ClassWithConstructor = { new (...args: any[]): T }; + /** * Checks whether given value's type is an instance of provided constructor. * {@link isInstanceOf}. @@ -148,7 +154,7 @@ export function isSyntheticEvent(wat: any): boolean { * @param base A constructor to be used in a check. * @returns A boolean representing the result. */ -export function isInstanceOf(wat: any, base: any): boolean { +export function isInstanceOf(wat: any, base: Constructor | ClassWithConstructor): wat is T { try { return wat instanceof base; } catch (_e) { From 5d7826af3c4c706d5f8d63b9ae2ac8cf5a09fb3b Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 4 Nov 2021 14:11:59 -0700 Subject: [PATCH 2/2] remove unnecessary typecasts --- packages/browser/src/eventbuilder.ts | 13 ++++++------- packages/integrations/src/ember.ts | 2 +- packages/integrations/src/extraerrordata.ts | 4 ++-- packages/nextjs/src/utils/instrumentServer.ts | 2 +- packages/nextjs/src/utils/withSentry.ts | 2 +- packages/node/src/backend.ts | 2 +- packages/node/src/handlers.ts | 2 +- packages/serverless/src/awslambda.ts | 2 +- packages/serverless/src/gcpfunction/http.ts | 2 +- packages/tracing/src/browser/request.ts | 2 +- packages/tracing/src/transaction.ts | 2 +- packages/utils/src/object.ts | 13 ++++++------- 12 files changed, 23 insertions(+), 25 deletions(-) diff --git a/packages/browser/src/eventbuilder.ts b/packages/browser/src/eventbuilder.ts index c3734c9c39e6..ef77a22ebcd8 100644 --- a/packages/browser/src/eventbuilder.ts +++ b/packages/browser/src/eventbuilder.ts @@ -65,15 +65,14 @@ export function eventFromUnknownInput( ): Event { let event: Event; - if (isErrorEvent(exception as ErrorEvent) && (exception as ErrorEvent).error) { + if (isErrorEvent(exception)) { // If it is an ErrorEvent with `error` property, extract it to get actual Error - const errorEvent = exception as ErrorEvent; // eslint-disable-next-line no-param-reassign - exception = errorEvent.error; - event = eventFromStacktrace(computeStackTrace(exception as Error)); + exception = exception.error; + event = eventFromStacktrace(computeStackTrace(exception)); return event; } - if (isDOMError(exception as DOMError) || isDOMException(exception as DOMException)) { + if (isDOMError(exception) || isDOMException(exception)) { // If it is a DOMError or DOMException (which are legacy APIs, but still supported in some browsers) // then we just extract the name, code, and message, as they don't provide anything else // https://developer.mozilla.org/en-US/docs/Web/API/DOMError @@ -90,9 +89,9 @@ export function eventFromUnknownInput( return event; } - if (isError(exception as Error)) { + if (isError(exception)) { // we have a real Error object, do nothing - event = eventFromStacktrace(computeStackTrace(exception as Error)); + event = eventFromStacktrace(computeStackTrace(exception)); return event; } if (isPlainObject(exception) || isEvent(exception)) { diff --git a/packages/integrations/src/ember.ts b/packages/integrations/src/ember.ts index 811e8b4852f8..c31c5aebb809 100644 --- a/packages/integrations/src/ember.ts +++ b/packages/integrations/src/ember.ts @@ -58,7 +58,7 @@ export class Ember implements Integration { getCurrentHub().withScope(scope => { if (isInstanceOf(reason, Error)) { scope.setExtra('context', 'Unhandled Promise error detected'); - getCurrentHub().captureException(reason, { originalException: reason as Error }); + getCurrentHub().captureException(reason, { originalException: reason }); } else { scope.setExtra('reason', reason); getCurrentHub().captureMessage('Unhandled Promise error detected'); diff --git a/packages/integrations/src/extraerrordata.ts b/packages/integrations/src/extraerrordata.ts index de01df6605c0..5fc3bb2f319f 100644 --- a/packages/integrations/src/extraerrordata.ts +++ b/packages/integrations/src/extraerrordata.ts @@ -97,7 +97,7 @@ export class ExtraErrorData implements Integration { continue; } const value = error[key]; - extraErrorInfo[key] = isError(value) ? (value as Error).toString() : value; + extraErrorInfo[key] = isError(value) ? value.toString() : value; } // Check if someone attached `toJSON` method to grab even more properties (eg. axios is doing that) @@ -106,7 +106,7 @@ export class ExtraErrorData implements Integration { for (const key of Object.keys(serializedError)) { const value = serializedError[key]; - extraErrorInfo[key] = isError(value) ? (value as Error).toString() : value; + extraErrorInfo[key] = isError(value) ? value.toString() : value; } } diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts index c05fa9cac226..6dd28b66c6d8 100644 --- a/packages/nextjs/src/utils/instrumentServer.ts +++ b/packages/nextjs/src/utils/instrumentServer.ts @@ -227,7 +227,7 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler { // If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision) let traceparentData; if (req.headers && isString(req.headers['sentry-trace'])) { - traceparentData = extractTraceparentData(req.headers['sentry-trace'] as string); + traceparentData = extractTraceparentData(req.headers['sentry-trace']); logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); } diff --git a/packages/nextjs/src/utils/withSentry.ts b/packages/nextjs/src/utils/withSentry.ts index f7792070f400..3f7a737904e7 100644 --- a/packages/nextjs/src/utils/withSentry.ts +++ b/packages/nextjs/src/utils/withSentry.ts @@ -39,7 +39,7 @@ export const withSentry = (origHandler: NextApiHandler): WrappedNextApiHandler = // If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision) let traceparentData; if (req.headers && isString(req.headers['sentry-trace'])) { - traceparentData = extractTraceparentData(req.headers['sentry-trace'] as string); + traceparentData = extractTraceparentData(req.headers['sentry-trace']); logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`); } diff --git a/packages/node/src/backend.ts b/packages/node/src/backend.ts index 8220d690ceb4..fea0407b412b 100644 --- a/packages/node/src/backend.ts +++ b/packages/node/src/backend.ts @@ -41,7 +41,7 @@ export class NodeBackend extends BaseBackend { const message = `Non-Error exception captured with keys: ${extractExceptionKeysForMessage(exception)}`; getCurrentHub().configureScope(scope => { - scope.setExtra('__serialized__', normalizeToSize(exception as Record)); + scope.setExtra('__serialized__', normalizeToSize(exception)); }); ex = (hint && hint.syntheticException) || new Error(message); diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index c8156896acc8..0aa67d34267a 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -56,7 +56,7 @@ export function tracingHandler(): ( // If there is a trace header set, we extract the data from it (parentSpanId, traceId, and sampling decision) let traceparentData; if (req.headers && isString(req.headers['sentry-trace'])) { - traceparentData = extractTraceparentData(req.headers['sentry-trace'] as string); + traceparentData = extractTraceparentData(req.headers['sentry-trace']); } const transaction = startTransaction( diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts index c2f3bed8f333..7b9f6608332f 100644 --- a/packages/serverless/src/awslambda.ts +++ b/packages/serverless/src/awslambda.ts @@ -256,7 +256,7 @@ export function wrapHandler( let traceparentData; const eventWithHeaders = event as { headers?: { [key: string]: string } }; if (eventWithHeaders.headers && isString(eventWithHeaders.headers['sentry-trace'])) { - traceparentData = extractTraceparentData(eventWithHeaders.headers['sentry-trace'] as string); + traceparentData = extractTraceparentData(eventWithHeaders.headers['sentry-trace']); } const transaction = startTransaction({ name: context.functionName, diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index d9996842b18c..30f22187ac25 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/packages/serverless/src/gcpfunction/http.ts @@ -53,7 +53,7 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial(val: T): T { - if (isPlainObject(val)) { - const obj = val as { [key: string]: any }; +export function dropUndefinedKeys(obj: T): T { + if (isPlainObject(obj)) { const rv: { [key: string]: any } = {}; for (const key of Object.keys(obj)) { if (typeof obj[key] !== 'undefined') { @@ -393,11 +392,11 @@ export function dropUndefinedKeys(val: T): T { return rv as T; } - if (Array.isArray(val)) { - return (val as any[]).map(dropUndefinedKeys) as any; + if (Array.isArray(obj)) { + return (obj as any[]).map(dropUndefinedKeys) as any; } - return val; + return obj; } /**