Throw exception when Performance custom trace API is provided with no…

…n-positive startTime or duration (#3866)
firebase · Oct 2, 2020 · 8728e1a · 8728e1a
1 parent e68c47e
commit 8728e1a
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 3 deletions.
diff --git a/.changeset/modern-parents-worry.md b/.changeset/modern-parents-worry.md
@@ -0,0 +1,6 @@
+---
+'@firebase/performance': patch
+'firebase': patch
+---
+
+Throws exception when startTime or duration is not positive value in `trace.record()` API.
diff --git a/packages-exp/performance-exp/src/resources/trace.test.ts b/packages-exp/performance-exp/src/resources/trace.test.ts
@@ -94,6 +94,16 @@ describe('Firebase Performance > trace', () => {
   });
 
   describe('#record', () => {
+    it('logs a custom trace with non-positive start time value', () => {
+      expect(() => trace.record(0, 20)).to.throw();
+      expect(() => trace.record(-100, 20)).to.throw();
+    });
+
+    it('logs a custom trace with non-positive duration value', () => {
+      expect(() => trace.record(1000, 0)).to.throw();
+      expect(() => trace.record(1000, -200)).to.throw();
+    });
+
     it('logs a trace without metrics or custom attributes', () => {
       trace.record(1, 20);
 

diff --git a/packages-exp/performance-exp/src/resources/trace.ts b/packages-exp/performance-exp/src/resources/trace.ts
@@ -134,6 +134,17 @@ export class Trace implements PerformanceTrace {
       attributes?: { [key: string]: string };
     }
   ): void {
+    if (startTime <= 0) {
+      throw ERROR_FACTORY.create(ErrorCode.NONPOSITIVE_TRACE_START_TIME, {
+        traceName: this.name
+      });
+    }
+    if (duration <= 0) {
+      throw ERROR_FACTORY.create(ErrorCode.NONPOSITIVE_TRACE_DURATION, {
+        traceName: this.name
+      });
+    }
+
     this.durationUs = Math.floor(duration * 1000);
     this.startTimeUs = Math.floor(startTime * 1000);
     if (options && options.attributes) {

diff --git a/packages-exp/performance-exp/src/utils/errors.ts b/packages-exp/performance-exp/src/utils/errors.ts
@@ -21,6 +21,8 @@ import { SERVICE, SERVICE_NAME } from '../constants';
 export const enum ErrorCode {
   TRACE_STARTED_BEFORE = 'trace started',
   TRACE_STOPPED_BEFORE = 'trace stopped',
+  NONPOSITIVE_TRACE_START_TIME = 'nonpositive trace startTime',
+  NONPOSITIVE_TRACE_DURATION = 'nonpositive trace duration',
   NO_WINDOW = 'no window',
   NO_APP_ID = 'no app id',
   NO_PROJECT_ID = 'no project id',
@@ -37,6 +39,10 @@ export const enum ErrorCode {
 const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
   [ErrorCode.TRACE_STARTED_BEFORE]: 'Trace {$traceName} was started before.',
   [ErrorCode.TRACE_STOPPED_BEFORE]: 'Trace {$traceName} is not running.',
+  [ErrorCode.NONPOSITIVE_TRACE_START_TIME]:
+    'Trace {$traceName} startTime should be positive.',
+  [ErrorCode.NONPOSITIVE_TRACE_DURATION]:
+    'Trace {$traceName} duration should be positive.',
   [ErrorCode.NO_WINDOW]: 'Window is not available.',
   [ErrorCode.NO_APP_ID]: 'App id is not available.',
   [ErrorCode.NO_PROJECT_ID]: 'Project id is not available.',
@@ -58,6 +64,8 @@ const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
 interface ErrorParams {
   [ErrorCode.TRACE_STARTED_BEFORE]: { traceName: string };
   [ErrorCode.TRACE_STOPPED_BEFORE]: { traceName: string };
+  [ErrorCode.NONPOSITIVE_TRACE_START_TIME]: { traceName: string };
+  [ErrorCode.NONPOSITIVE_TRACE_DURATION]: { traceName: string };
   [ErrorCode.INVALID_ATTRIBUTE_NAME]: { attributeName: string };
   [ErrorCode.INVALID_ATTRIBUTE_VALUE]: { attributeValue: string };
   [ErrorCode.INVALID_CUSTOM_METRIC_NAME]: { customMetricName: string };

diff --git a/packages/performance/src/resources/trace.test.ts b/packages/performance/src/resources/trace.test.ts
@@ -71,10 +71,14 @@ describe('Firebase Performance > trace', () => {
   });
 
   describe('#record', () => {
-    it('logs a trace without metrics or custom attributes', () => {
-      trace.record(1, 20);
+    it('logs a custom trace with non-positive start time value', () => {
+      expect(() => trace.record(0, 20)).to.throw();
+      expect(() => trace.record(-100, 20)).to.throw();
+    });
 
-      expect((perfLogger.logTrace as any).calledOnceWith(trace)).to.be.true;
+    it('logs a custom trace with non-positive duration value', () => {
+      expect(() => trace.record(1000, 0)).to.throw();
+      expect(() => trace.record(1000, -200)).to.throw();
     });
 
     it('logs a trace with metrics', () => {

diff --git a/packages/performance/src/resources/trace.ts b/packages/performance/src/resources/trace.ts
@@ -131,6 +131,17 @@ export class Trace implements PerformanceTrace {
       attributes?: { [key: string]: string };
     }
   ): void {
+    if (startTime <= 0) {
+      throw ERROR_FACTORY.create(ErrorCode.NONPOSITIVE_TRACE_START_TIME, {
+        traceName: this.name
+      });
+    }
+    if (duration <= 0) {
+      throw ERROR_FACTORY.create(ErrorCode.NONPOSITIVE_TRACE_DURATION, {
+        traceName: this.name
+      });
+    }
+
     this.durationUs = Math.floor(duration * 1000);
     this.startTimeUs = Math.floor(startTime * 1000);
     if (options && options.attributes) {

diff --git a/packages/performance/src/utils/errors.ts b/packages/performance/src/utils/errors.ts
@@ -21,6 +21,8 @@ import { SERVICE, SERVICE_NAME } from '../constants';
 export const enum ErrorCode {
   TRACE_STARTED_BEFORE = 'trace started',
   TRACE_STOPPED_BEFORE = 'trace stopped',
+  NONPOSITIVE_TRACE_START_TIME = 'nonpositive trace startTime',
+  NONPOSITIVE_TRACE_DURATION = 'nonpositive trace duration',
   NO_WINDOW = 'no window',
   NO_APP_ID = 'no app id',
   NO_PROJECT_ID = 'no project id',
@@ -37,6 +39,10 @@ export const enum ErrorCode {
 const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
   [ErrorCode.TRACE_STARTED_BEFORE]: 'Trace {$traceName} was started before.',
   [ErrorCode.TRACE_STOPPED_BEFORE]: 'Trace {$traceName} is not running.',
+  [ErrorCode.NONPOSITIVE_TRACE_START_TIME]:
+    'Trace {$traceName} startTime should be positive.',
+  [ErrorCode.NONPOSITIVE_TRACE_DURATION]:
+    'Trace {$traceName} duration should be positive.',
   [ErrorCode.NO_WINDOW]: 'Window is not available.',
   [ErrorCode.NO_APP_ID]: 'App id is not available.',
   [ErrorCode.NO_PROJECT_ID]: 'Project id is not available.',
@@ -58,6 +64,8 @@ const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
 interface ErrorParams {
   [ErrorCode.TRACE_STARTED_BEFORE]: { traceName: string };
   [ErrorCode.TRACE_STOPPED_BEFORE]: { traceName: string };
+  [ErrorCode.NONPOSITIVE_TRACE_START_TIME]: { traceName: string };
+  [ErrorCode.NONPOSITIVE_TRACE_DURATION]: { traceName: string };
   [ErrorCode.INVALID_ATTRIBUTE_NAME]: { attributeName: string };
   [ErrorCode.INVALID_ATTRIBUTE_VALUE]: { attributeValue: string };
   [ErrorCode.INVALID_CUSTOM_METRIC_NAME]: { customMetricName: string };