Browse Source

feat(ui): Stop capturing request errors to Sentry (#34653)

These errors seemed to be mostly in-actonable and added a lot of noise to our Issues stream.
Billy Vong 2 years ago
parent
commit
41c60c5345
3 changed files with 61 additions and 146 deletions
  1. 61 109
      static/app/api.tsx
  2. 0 5
      static/app/bootstrap/initializeSdk.tsx
  3. 0 32
      static/app/utils/apiSentryClient.tsx

+ 61 - 109
static/app/api.tsx

@@ -1,4 +1,5 @@
 import {browserHistory} from 'react-router';
+import * as Sentry from '@sentry/react';
 import Cookies from 'js-cookie';
 import isUndefined from 'lodash/isUndefined';
 import * as qs from 'query-string';
@@ -11,7 +12,6 @@ import {
   SUPERUSER_REQUIRED,
 } from 'sentry/constants/apiErrorCodes';
 import {metric} from 'sentry/utils/analytics';
-import {run} from 'sentry/utils/apiSentryClient';
 import getCsrfToken from 'sentry/utils/getCsrfToken';
 import {uniqueId} from 'sentry/utils/guid';
 import createRequestError from 'sentry/utils/requestError/createRequestError';
@@ -140,13 +140,11 @@ function buildRequestUrl(baseUrl: string, path: string, query: RequestOptions['q
   try {
     params = qs.stringify(query ?? []);
   } catch (err) {
-    run(Sentry =>
-      Sentry.withScope(scope => {
-        scope.setExtra('path', path);
-        scope.setExtra('query', query);
-        Sentry.captureException(err);
-      })
-    );
+    Sentry.withScope(scope => {
+      scope.setExtra('path', path);
+      scope.setExtra('query', query);
+      Sentry.captureException(err);
+    });
     throw err;
   }
 
@@ -362,8 +360,6 @@ export class Client {
 
     metric.mark({name: startMarker});
 
-    const errorObject = new Error();
-
     /**
      * Called when the request completes with a 2xx status
      */
@@ -400,35 +396,6 @@ export class Client {
         data: {status: resp?.status},
       });
 
-      if (
-        resp &&
-        resp.status !== 0 &&
-        resp.status !== 404 &&
-        errorThrown !== 'Request was aborted'
-      ) {
-        run(Sentry =>
-          Sentry.withScope(scope => {
-            // `requestPromise` can pass its error object
-            const preservedError = options.preservedError ?? errorObject;
-
-            const errorObjectToUse = createRequestError(
-              resp,
-              preservedError.stack,
-              method,
-              path
-            );
-
-            errorObjectToUse.removeFrames(3);
-
-            // Setting this to warning because we are going to capture all failed requests
-            scope.setLevel('warning');
-            scope.setTag('http.statusCode', String(resp.status));
-            scope.setTag('error.reason', errorThrown);
-            Sentry.captureException(errorObjectToUse);
-          })
-        );
-      }
-
       this.handleRequestError(
         {id, path, requestOptions: options},
         resp,
@@ -478,86 +445,71 @@ export class Client {
 
     // XXX(epurkhiser): We migrated off of jquery, so for now we have a
     // compatibility layer which mimics that of the jquery response objects.
-    fetchRequest
-      .then(async response => {
-        // The Response's body can only be resolved/used at most once.
-        // So we clone the response so we can resolve the body content as text content.
-        // Response objects need to be cloned before its body can be used.
-        const responseClone = response.clone();
-
-        let responseJSON: any;
-        let responseText: any;
+    fetchRequest.then(async response => {
+      // The Response's body can only be resolved/used at most once.
+      // So we clone the response so we can resolve the body content as text content.
+      // Response objects need to be cloned before its body can be used.
+      const responseClone = response.clone();
+
+      let responseJSON: any;
+      let responseText: any;
+
+      const {status, statusText} = response;
+      let {ok} = response;
+      let errorReason = 'Request not OK'; // the default error reason
+
+      // Try to get text out of the response no matter the status
+      try {
+        responseText = await response.text();
+      } catch (error) {
+        ok = false;
+        if (error.name === 'AbortError') {
+          errorReason = 'Request was aborted';
+        } else {
+          errorReason = error.toString();
+        }
+      }
 
-        const {status, statusText} = response;
-        let {ok} = response;
-        let errorReason = 'Request not OK'; // the default error reason
+      const responseContentType = response.headers.get('content-type');
+      const isResponseJSON = responseContentType?.includes('json');
 
-        // Try to get text out of the response no matter the status
+      const isStatus3XX = status >= 300 && status < 400;
+      if (status !== 204 && !isStatus3XX) {
         try {
-          responseText = await response.text();
+          responseJSON = await responseClone.json();
         } catch (error) {
-          ok = false;
           if (error.name === 'AbortError') {
+            ok = false;
             errorReason = 'Request was aborted';
-          } else {
-            errorReason = error.toString();
+          } else if (isResponseJSON && error instanceof SyntaxError) {
+            // If the MIME type is `application/json` but decoding failed,
+            // this should be an error.
+            ok = false;
+            errorReason = 'JSON parse error';
           }
         }
+      }
 
-        const responseContentType = response.headers.get('content-type');
-        const isResponseJSON = responseContentType?.includes('json');
-
-        const isStatus3XX = status >= 300 && status < 400;
-        if (status !== 204 && !isStatus3XX) {
-          try {
-            responseJSON = await responseClone.json();
-          } catch (error) {
-            if (error.name === 'AbortError') {
-              ok = false;
-              errorReason = 'Request was aborted';
-            } else if (isResponseJSON && error instanceof SyntaxError) {
-              // If the MIME type is `application/json` but decoding failed,
-              // this should be an error.
-              ok = false;
-              errorReason = 'JSON parse error';
-            }
-          }
-        }
-
-        const responseMeta: ResponseMeta = {
-          status,
-          statusText,
-          responseJSON,
-          responseText,
-          getResponseHeader: (header: string) => response.headers.get(header),
-        };
-
-        // Respect the response content-type header
-        const responseData = isResponseJSON ? responseJSON : responseText;
-
-        if (ok) {
-          successHandler(responseMeta, statusText, responseData);
-        } else {
-          globalErrorHandlers.forEach(handler => handler(responseMeta));
-          errorHandler(responseMeta, statusText, errorReason);
-        }
-
-        completeHandler(responseMeta, statusText);
-      })
-      .catch(err => {
-        // Aborts are expected
-        if (err?.name === 'AbortError') {
-          return;
-        }
+      const responseMeta: ResponseMeta = {
+        status,
+        statusText,
+        responseJSON,
+        responseText,
+        getResponseHeader: (header: string) => response.headers.get(header),
+      };
+
+      // Respect the response content-type header
+      const responseData = isResponseJSON ? responseJSON : responseText;
+
+      if (ok) {
+        successHandler(responseMeta, statusText, responseData);
+      } else {
+        globalErrorHandlers.forEach(handler => handler(responseMeta));
+        errorHandler(responseMeta, statusText, errorReason);
+      }
 
-        // The request failed for other reason
-        run(Sentry =>
-          Sentry.withScope(scope => {
-            scope.setLevel('warning');
-            Sentry.captureException(err);
-          })
-        );
-      });
+      completeHandler(responseMeta, statusText);
+    });
 
     const request = new Request(fetchRequest, aborter);
     this.activeRequests[id] = request;

+ 0 - 5
static/app/bootstrap/initializeSdk.tsx

@@ -7,7 +7,6 @@ import {_browserPerformanceTimeOriginMode} from '@sentry/utils';
 
 import {DISABLE_RR_WEB, SENTRY_RELEASE_VERSION, SPA_DSN} from 'sentry/constants';
 import {Config} from 'sentry/types';
-import {init as initApiSentryClient} from 'sentry/utils/apiSentryClient';
 import {LongTaskObserver} from 'sentry/utils/performanceForSentry';
 
 /**
@@ -61,10 +60,6 @@ function getSentryIntegrations(hasReplays: boolean = false, routes?: Function) {
  * entrypoints require this.
  */
 export function initializeSdk(config: Config, {routes}: {routes?: Function} = {}) {
-  if (config.dsn_requests) {
-    initApiSentryClient(config.dsn_requests);
-  }
-
   const {apmSampling, sentryConfig, userIdentity} = config;
   const tracesSampleRate = apmSampling ?? 0;
 

+ 0 - 32
static/app/utils/apiSentryClient.tsx

@@ -1,32 +0,0 @@
-import {
-  BrowserClient,
-  defaultIntegrations,
-  defaultStackParser,
-  Hub,
-  makeFetchTransport,
-} from '@sentry/react';
-
-let hub: Hub | undefined;
-
-function init(dsn: string) {
-  // This client is used to track all API requests that use `app/api`
-  // This is a bit noisy so we don't want it in the main project (yet)
-  const client = new BrowserClient({
-    dsn,
-    transport: makeFetchTransport,
-    stackParser: defaultStackParser,
-    integrations: defaultIntegrations,
-  });
-
-  hub = new Hub(client);
-}
-
-const run: Hub['run'] = cb => {
-  if (!hub) {
-    return;
-  }
-
-  hub.run(cb);
-};
-
-export {init, run};