fix: Several fixes (#1671)

Author: HazAT

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+- [core]: Invert logger logic the explicitly enable it.
+- [hub]: Require `domain` in `getCurrentHub` in try/catch - Fixed #1670
+- [hub]: Removed exposed getter on the Scope.
+
 ## 4.2.0
 
 - [browser] fix: Make `addBreadcrumb` sync internally, `beforeBreadcrumb` is now only sync

packages/core/src/integrations/dedupe.ts

@@ -1,7 +1,7 @@
 import { addGlobalEventProcessor, getCurrentHub } from '@sentry/hub';
 import { Integration, SentryEvent, SentryException, StackFrame } from '@sentry/types';
+import { logger } from '@sentry/utils/logger';
 import { getEventDescription } from '@sentry/utils/misc';
-import { logger } from '../logger';
 
 /** Deduplication filter */
 export class Dedupe implements Integration {

packages/core/src/logger.ts

@@ -16,8 +16,8 @@ export interface ClientClass<F extends Client, O extends Options> {
  * @returns The installed and bound client instance.
  */
 export function initAndBind<F extends Client, O extends Options>(clientClass: ClientClass<F, O>, options: O): void {
-  if (!options.debug) {
-    logger.disable();
+  if (options.debug === true) {
+    logger.enable();
   }
 
   if (getCurrentHub().getClient()) {

packages/core/src/sdk.ts

@@ -101,65 +101,65 @@ describe('BaseClient', () => {
       const scope = new Scope();
       scope.addBreadcrumb({ message: 'hello' }, 100);
       client.addBreadcrumb({ message: 'world' }, undefined, scope);
-      expect(scope.getBreadcrumbs()[1].message).toBe('world');
+      expect((scope as any).breadcrumbs[1].message).toBe('world');
     });
 
     test('adds a timestamp to new breadcrumbs', () => {
       const client = new TestClient({});
       const scope = new Scope();
       scope.addBreadcrumb({ message: 'hello' }, 100);
       client.addBreadcrumb({ message: 'world' }, undefined, scope);
-      expect(scope.getBreadcrumbs()[1].timestamp).toBeGreaterThan(1);
+      expect((scope as any).breadcrumbs[1].timestamp).toBeGreaterThan(1);
     });
 
     test('discards breadcrumbs beyond maxBreadcrumbs', () => {
       const client = new TestClient({ maxBreadcrumbs: 1 });
       const scope = new Scope();
       scope.addBreadcrumb({ message: 'hello' }, 100);
       client.addBreadcrumb({ message: 'world' }, undefined, scope);
-      expect(scope.getBreadcrumbs().length).toBe(1);
-      expect(scope.getBreadcrumbs()[0].message).toBe('world');
+      expect((scope as any).breadcrumbs.length).toBe(1);
+      expect((scope as any).breadcrumbs[0].message).toBe('world');
     });
 
     test('allows concurrent updates', () => {
       const client = new TestClient({});
       const scope = new Scope();
       client.addBreadcrumb({ message: 'hello' }, undefined, scope);
       client.addBreadcrumb({ message: 'world' }, undefined, scope);
-      expect(scope.getBreadcrumbs()).toHaveLength(2);
+      expect((scope as any).breadcrumbs).toHaveLength(2);
     });
 
     test('calls beforeBreadcrumb and adds the breadcrumb without any changes', () => {
       const beforeBreadcrumb = jest.fn(breadcrumb => breadcrumb);
       const client = new TestClient({ beforeBreadcrumb });
       const scope = new Scope();
       client.addBreadcrumb({ message: 'hello' }, undefined, scope);
-      expect(scope.getBreadcrumbs()[0].message).toBe('hello');
+      expect((scope as any).breadcrumbs[0].message).toBe('hello');
     });
 
     test('calls beforeBreadcrumb and uses the new one', () => {
       const beforeBreadcrumb = jest.fn(() => ({ message: 'changed' }));
       const client = new TestClient({ beforeBreadcrumb });
       const scope = new Scope();
       client.addBreadcrumb({ message: 'hello' }, undefined, scope);
-      expect(scope.getBreadcrumbs()[0].message).toBe('changed');
+      expect((scope as any).breadcrumbs[0].message).toBe('changed');
     });
 
     test('calls beforeBreadcrumb and discards the breadcrumb when returned null', () => {
       const beforeBreadcrumb = jest.fn(() => null);
       const client = new TestClient({ beforeBreadcrumb });
       const scope = new Scope();
       client.addBreadcrumb({ message: 'hello' }, undefined, scope);
-      expect(scope.getBreadcrumbs().length).toBe(0);
+      expect((scope as any).breadcrumbs.length).toBe(0);
     });
 
     test('calls beforeBreadcrumb gets an access to a hint as a second argument', () => {
       const beforeBreadcrumb = jest.fn((breadcrumb, hint) => ({ ...breadcrumb, data: hint.data }));
       const client = new TestClient({ beforeBreadcrumb });
       const scope = new Scope();
       client.addBreadcrumb({ message: 'hello' }, { data: 'someRandomThing' }, scope);
-      expect(scope.getBreadcrumbs()[0].message).toBe('hello');
-      expect(scope.getBreadcrumbs()[0].data).toBe('someRandomThing');
+      expect((scope as any).breadcrumbs[0].message).toBe('hello');
+      expect((scope as any).breadcrumbs[0].data).toBe('someRandomThing');
     });
   });
 

packages/core/test/lib/base.test.ts

@@ -9,7 +9,6 @@ import {
 } from '@sentry/types';
 import { logger } from '@sentry/utils/logger';
 import { getGlobalObject, uuid4 } from '@sentry/utils/misc';
-import * as domain from 'domain';
 import { Carrier, Layer } from './interfaces';
 import { Scope } from './scope';
 
@@ -335,6 +334,7 @@ export function getCurrentHub(): Hub {
 
   // Prefer domains over global if they are there
   try {
+    const domain = require('domain');
     const activeDomain = domain.active;
 
     // If there no active domain, just return global hub

packages/hub/src/hub.ts

@@ -148,36 +148,6 @@ export class Scope {
     return newScope;
   }
 
-  /** Returns tags. */
-  public getTags(): { [key: string]: string } {
-    return this.tags;
-  }
-
-  /** Returns extra. */
-  public getExtra(): { [key: string]: any } {
-    return this.extra;
-  }
-
-  /** Returns extra. */
-  public getUser(): User {
-    return this.user;
-  }
-
-  /** Returns fingerprint. */
-  public getFingerprint(): string[] | undefined {
-    return this.fingerprint;
-  }
-
-  /** Returns breadcrumbs. */
-  public getBreadcrumbs(): Breadcrumb[] {
-    return this.breadcrumbs;
-  }
-
-  /** Returns level. */
-  public getLevel(): Severity | undefined {
-    return this.level;
-  }
-
   /** Clears the current scope and resets its properties. */
   public clear(): void {
     this.breadcrumbs = [];

packages/hub/src/scope.ts

@@ -76,7 +76,7 @@ describe('Hub', () => {
     hub.pushScope();
     expect(hub.getStack()).toHaveLength(2);
     expect(hub.getStack()[1].scope).not.toBe(localScope);
-    expect((hub.getStack()[1].scope as Scope).getExtra()).toEqual({ a: 'b' });
+    expect(((hub.getStack()[1].scope as Scope) as any).extra).toEqual({ a: 'b' });
   });
 
   test('pushScope inherit client', () => {
@@ -241,7 +241,7 @@ describe('Hub', () => {
     localScope.setExtra('a', 'b');
     const hub = new Hub({ a: 'b' }, localScope);
     hub.configureScope(confScope => {
-      expect(confScope.getExtra()).toEqual({ a: 'b' });
+      expect((confScope as any).extra).toEqual({ a: 'b' });
     });
   });
 

packages/hub/test/hub.test.ts

@@ -10,59 +10,59 @@ describe('Scope', () => {
   test('fingerprint', () => {
     const scope = new Scope();
     scope.setFingerprint(['abcd']);
-    expect(scope.getFingerprint()).toEqual(['abcd']);
+    expect((scope as any).fingerprint).toEqual(['abcd']);
   });
 
   test('extra', () => {
     const scope = new Scope();
     scope.setExtra('a', 1);
-    expect(scope.getExtra()).toEqual({ a: 1 });
+    expect((scope as any).extra).toEqual({ a: 1 });
   });
 
   test('tags', () => {
     const scope = new Scope();
     scope.setTag('a', 'b');
-    expect(scope.getTags()).toEqual({ a: 'b' });
+    expect((scope as any).tags).toEqual({ a: 'b' });
   });
 
   test('user', () => {
     const scope = new Scope();
     scope.setUser({ id: '1' });
-    expect(scope.getUser()).toEqual({ id: '1' });
+    expect((scope as any).user).toEqual({ id: '1' });
   });
 
   test('breadcrumbs', () => {
     const scope = new Scope();
     scope.addBreadcrumb({ message: 'test' }, 100);
-    expect(scope.getBreadcrumbs()).toEqual([{ message: 'test' }]);
+    expect((scope as any).breadcrumbs).toEqual([{ message: 'test' }]);
   });
 
   test('level', () => {
     const scope = new Scope();
     scope.setLevel(Severity.Critical);
-    expect(scope.getLevel()).toEqual(Severity.Critical);
+    expect((scope as any).level).toEqual(Severity.Critical);
   });
 
   test('chaining', () => {
     const scope = new Scope();
     scope.setLevel(Severity.Critical).setUser({ id: '1' });
-    expect(scope.getLevel()).toEqual(Severity.Critical);
-    expect(scope.getUser()).toEqual({ id: '1' });
+    expect((scope as any).level).toEqual(Severity.Critical);
+    expect((scope as any).user).toEqual({ id: '1' });
   });
 
   test('basic inheritance', () => {
     const parentScope = new Scope();
     parentScope.setExtra('a', 1);
     const scope = Scope.clone(parentScope);
-    expect(parentScope.getExtra()).toEqual(scope.getExtra());
+    expect((parentScope as any).extra).toEqual((scope as any).extra);
   });
 
   test('parent changed inheritance', () => {
     const parentScope = new Scope();
     const scope = Scope.clone(parentScope);
     parentScope.setExtra('a', 2);
-    expect(scope.getExtra()).toEqual({});
-    expect(parentScope.getExtra()).toEqual({ a: 2 });
+    expect((scope as any).extra).toEqual({});
+    expect((parentScope as any).extra).toEqual({ a: 2 });
   });
 
   test('child override inheritance', () => {
@@ -71,8 +71,8 @@ describe('Scope', () => {
 
     const scope = Scope.clone(parentScope);
     scope.setExtra('a', 2);
-    expect(parentScope.getExtra()).toEqual({ a: 1 });
-    expect(scope.getExtra()).toEqual({ a: 2 });
+    expect((parentScope as any).extra).toEqual({ a: 1 });
+    expect((scope as any).extra).toEqual({ a: 2 });
   });
 
   test('listeners', () => {
@@ -142,9 +142,9 @@ describe('Scope', () => {
     scope.setUser({ id: '1' });
     scope.setFingerprint(['abcd']);
     scope.addBreadcrumb({ message: 'test' }, 100);
-    expect(scope.getExtra()).toEqual({ a: 2 });
+    expect((scope as any).extra).toEqual({ a: 2 });
     scope.clear();
-    expect(scope.getExtra()).toEqual({});
+    expect((scope as any).extra).toEqual({});
   });
 
   test('addEventProcessor', async done => {

packages/hub/test/scope.test.ts

@@ -10,7 +10,7 @@ class Logger {
 
   /** JSDoc */
   public constructor() {
-    this.enabled = true;
+    this.enabled = false;
   }
 
   /** JSDoc */