chore(various): Tiny fixes (#3097)

getsentry · Dec 4, 2020 · 22ecbcd · 22ecbcd
1 parent c55294f
commit 22ecbcd
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 9 deletions.
diff --git a/packages/browser/src/transports/base.ts b/packages/browser/src/transports/base.ts
@@ -67,7 +67,7 @@ export abstract class BaseTransport implements Transport {
      * https://developer.mozilla.org/en-US/docs/Web/API/Headers/get
      */
     const limited = this._handleRateLimit(headers);
-    if (limited) logger.warn(`Too many requests, backing off till: ${this._disabledUntil(requestType)}`);
+    if (limited) logger.warn(`Too many requests, backing off until: ${this._disabledUntil(requestType)}`);
 
     if (status === Status.Success) {
       resolve({ status });
@@ -100,6 +100,16 @@ export abstract class BaseTransport implements Transport {
     const raHeader = headers['retry-after'];
 
     if (rlHeader) {
+      // rate limit headers are of the form
+      //     <header>,<header>,..
+      // where each <header> is of the form
+      //     <retry_after>: <categories>: <scope>: <reason_code>
+      // where
+      //     <retry_after> is a delay in ms
+      //     <categories> is the event type(s) (error, transaction, etc) being rate limited and is of the form
+      //         <category>;<category>;...
+      //     <scope> is what's being limited (org, project, or key) - ignored by SDK
+      //     <reason_code> is an arbitrary string like "org_quota" - ignored by SDK
       for (const limit of rlHeader.trim().split(',')) {
         const parameters = limit.split(':', 2);
         const headerDelay = parseInt(parameters[0], 10);

diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts
@@ -61,7 +61,13 @@ export class FetchTransport extends BaseTransport {
               'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'),
               'retry-after': response.headers.get('Retry-After'),
             };
-            this._handleResponse({ requestType: sentryRequest.type, response, headers, resolve, reject });
+            this._handleResponse({
+              requestType: sentryRequest.type,
+              response,
+              headers,
+              resolve,
+              reject,
+            });
           })
           .catch(reject);
       }),

diff --git a/packages/node/src/transports/base.ts b/packages/node/src/transports/base.ts
@@ -12,7 +12,7 @@ import { SDK_NAME, SDK_VERSION } from '../version';
  * Internal used interface for typescript.
  * @hidden
  */
-export interface HTTPRequest {
+export interface HTTPModule {
   /**
    * Request wrapper
    * @param options These are {@see TransportOptions}
@@ -37,7 +37,7 @@ export interface HTTPRequest {
 /** Base Transport class implementation */
 export abstract class BaseTransport implements Transport {
   /** The Agent used for corresponding transport */
-  public module?: HTTPRequest;
+  public module?: HTTPModule;
 
   /** The Agent used for corresponding transport */
   public client?: http.Agent | https.Agent;
@@ -96,7 +96,7 @@ export abstract class BaseTransport implements Transport {
   }
 
   /** JSDoc */
-  protected async _sendWithModule(httpModule: HTTPRequest, event: Event): Promise<Response> {
+  protected async _sendWithModule(httpModule: HTTPModule, event: Event): Promise<Response> {
     if (new Date(Date.now()) < this._disabledUntil) {
       return Promise.reject(new SentryError(`Transport locked till ${this._disabledUntil} due to too many requests.`));
     }

diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts
@@ -43,6 +43,8 @@ describe('tracing', () => {
     // TODO: For some reason in node 6 two request spans are appearing. Once we stop testing against it, this can go
     // back to being `toEqual()`.
     expect(spans.length).toBeGreaterThanOrEqual(2);
+
+    // our span is at index 1 because the transaction itself is at index 0
     expect(spans[1].description).toEqual('GET http://dogs.are.great/');
     expect(spans[1].op).toEqual('request');
   });

diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts
@@ -195,7 +195,7 @@ describe('Span', () => {
         expect(spy.mock.calls[0][0].contexts.trace).toEqual(transaction.getTraceContext());
       });
 
-      test('span child limit', () => {
+      test('maxSpans correctly limits number of spans', () => {
         const _hub = new Hub(
           new BrowserClient({
             _experiments: { maxSpans: 3 },
@@ -212,14 +212,14 @@ describe('Span', () => {
         expect(spy.mock.calls[0][0].spans).toHaveLength(3);
       });
 
-      test('if we sampled the transaction we do not want any children', () => {
+      test('no span recorder created if transaction.sampled is false', () => {
         const _hub = new Hub(
           new BrowserClient({
-            tracesSampleRate: 0,
+            tracesSampleRate: 1,
           }),
         );
         const spy = jest.spyOn(_hub as any, 'captureEvent') as any;
-        const transaction = _hub.startTransaction({ name: 'test' });
+        const transaction = _hub.startTransaction({ name: 'test', sampled: false });
         for (let i = 0; i < 10; i++) {
           const child = transaction.startChild();
           child.finish();