feat: update reporting functions to include a partial context in the error (#28)

* feat: update report error and warning fn

* update tests

* add transition to draft to active reporting options

* address code review comments

* update second arg to reporting function

* remove ts expect and switch to cast

* feat: update report error and warning fn

* update tests

* add transition to draft to active reporting options

* address code review comments

* update second arg to reporting function

* fix: improvements to error reporting and warning

Co-authored-by: Cynthia Ma <[email protected]>

* test: partially update test

* test: update failing tests

* Update src/v3/TraceManager.ts

Co-authored-by: Evan Jenkins <[email protected]>

---------

Co-authored-by: Bazyli Brzoska <[email protected]>
Co-authored-by: Evan Jenkins <[email protected]>

Author: 3 people

src/stories/mockComponentsv3/traceManager.tsx

@@ -10,9 +10,9 @@ export const traceManager = new TraceManager({
   },
   // eslint-disable-next-line no-magic-numbers
   generateId: () => Math.random().toString(36).slice(2),
-  reportErrorFn: (error) => {
+  reportErrorFn: (error, currentTraceContext) => {
     // eslint-disable-next-line no-console
-    console.error(error)
+    console.error(error, currentTraceContext)
   },
 })
 

src/v3/Trace.ts

@@ -27,12 +27,14 @@ import type {
   CompleteTraceDefinition,
   DraftTraceContext,
   RelationSchemasBase,
+  ReportErrorFn,
   TraceContext,
   TraceDefinitionModifications,
   TraceInterruptionReason,
   TraceInterruptionReasonForInvalidTraces,
   TraceManagerUtilities,
   TraceModifications,
+  TransitionDraftOptions,
 } from './types'
 import { INVALID_TRACE_INTERRUPTION_REASONS } from './types'
 import type {
@@ -932,8 +934,12 @@ export class Trace<
     VariantsT
   > {
     if (!this.input.relatedTo) {
-      throw new Error(
-        "Tried to access trace's activeInput, but the trace was never provided a 'relatedTo' input value",
+      this.traceUtilities.reportErrorFn(
+        new Error(
+          "Tried to access trace's activeInput, but the trace was never provided a 'relatedTo' input value",
+        ),
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        this as Trace<any, RelationSchemasT, any>,
       )
     }
     return this.input as ActiveTraceConfig<
@@ -1074,6 +1080,8 @@ export class Trace<
             input.variant
           }. Must be one of: ${Object.keys(definition.variants).join(', ')}`,
         ),
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        this as Trace<any, RelationSchemasT, any>,
       )
     }
 
@@ -1224,21 +1232,76 @@ export class Trace<
       RelationSchemasT,
       VariantsT
     >,
-  ) {
+    {
+      previouslyActivatedBehavior = 'warn-and-continue',
+      invalidRelatedToBehavior = 'warn-and-continue',
+    }: TransitionDraftOptions = {},
+  ): void {
+    const { isDraft } = this
+    let reportPreviouslyActivated: ReportErrorFn<RelationSchemasT>
+    let overwriteDraft = true
+    switch (previouslyActivatedBehavior) {
+      case 'error':
+        reportPreviouslyActivated = this.traceUtilities.reportErrorFn
+        overwriteDraft = false
+        break
+      case 'error-and-continue':
+        reportPreviouslyActivated = this.traceUtilities.reportErrorFn
+        break
+      default:
+        reportPreviouslyActivated = this.traceUtilities.reportWarningFn
+        break
+    }
+
+    // this is an already initialized active trace, do nothing:
+    if (!isDraft) {
+      reportPreviouslyActivated(
+        new Error(
+          `You are trying to activate a trace that has already been activated before (${this.definition.name}).`,
+        ),
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        this as Trace<any, RelationSchemasT, any>,
+      )
+      if (!overwriteDraft) {
+        return
+      }
+    }
+
+    let reportValidationError: ReportErrorFn<RelationSchemasT>
+    let useInvalidRelatedTo = true
+    switch (invalidRelatedToBehavior) {
+      case 'error':
+        reportValidationError = this.traceUtilities.reportErrorFn
+        useInvalidRelatedTo = false
+        break
+      case 'error-and-continue':
+        reportValidationError = this.traceUtilities.reportErrorFn
+        break
+      default:
+        reportValidationError = this.traceUtilities.reportWarningFn
+        break
+    }
+
     const { attributes } = this.input
 
     const { relatedTo, errors } = validateAndCoerceRelatedToAgainstSchema(
       inputAndDefinitionModifications.relatedTo,
       this.definition.relationSchema,
     )
+
     if (errors.length > 0) {
-      this.traceUtilities.reportWarningFn(
+      reportValidationError(
         new Error(
           `Invalid relatedTo value: ${JSON.stringify(
             inputAndDefinitionModifications.relatedTo,
           )}. ${errors.join(', ')}`,
         ),
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        this as Trace<any, RelationSchemasT, any>,
       )
+      if (!useInvalidRelatedTo) {
+        return
+      }
     }
 
     this.activeInput = {
@@ -1253,7 +1316,11 @@ export class Trace<
     this.applyDefinitionModifications(inputAndDefinitionModifications)
 
     this.wasActivated = true
-    this.stateMachine.emit('onMakeActive', undefined)
+
+    if (isDraft) {
+      // we might already be active in which case we would have issued a warning earlier in this method
+      this.stateMachine.emit('onMakeActive', undefined)
+    }
   }
 
   /**

src/v3/TraceManager.ts

@@ -12,7 +12,9 @@ import type {
   AllPossibleTraceContexts,
   CompleteTraceDefinition,
   ComputedValueDefinitionInput,
+  DraftTraceContext,
   RelationSchemasBase,
+  ReportErrorFn,
   SpanDeduplicationStrategy,
   TraceDefinition,
   TraceManagerConfig,
@@ -40,7 +42,7 @@ export class TraceManager<
     configInput: Omit<
       TraceManagerConfig<RelationSchemasT>,
       'reportWarningFn'
-    > & { reportWarningFn?: (warning: Error) => void },
+    > & { reportWarningFn?: ReportErrorFn<RelationSchemasT> },
   ) {
     this.utilities = {
       // by default noop for warnings
@@ -95,12 +97,6 @@ export class TraceManager<
       VariantsT
     >(traceDefinition.requiredSpans)
 
-    if (!requiredSpans) {
-      throw new Error(
-        'requiredSpans must be defined, as a trace will never end otherwise',
-      )
-    }
-
     const labelMatching = traceDefinition.labelMatching
       ? convertLabelMatchersToFns(traceDefinition.labelMatching)
       : undefined
@@ -180,7 +176,11 @@ export class TraceManager<
       VariantsT
     > = {
       ...traceDefinition,
-      requiredSpans,
+      requiredSpans:
+        requiredSpans ??
+        [
+          // lack of requiredSpan is invalid, but we warn about it below
+        ],
       debounceOnSpans,
       interruptOnSpans,
       suppressErrorStatusPropagationOnSpans,
@@ -191,6 +191,18 @@ export class TraceManager<
         this.utilities.relationSchemas[traceDefinition.relationSchemaName],
     }
 
+    if (!requiredSpans) {
+      this.utilities.reportErrorFn(
+        new Error(
+          'requiredSpans must be defined along with the trace, as a trace can only end in an interrupted state otherwise',
+        ),
+        { definition: completeTraceDefinition } as Partial<
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          DraftTraceContext<any, RelationSchemasT, any>
+        >,
+      )
+    }
+
     return new Tracer(completeTraceDefinition, this.utilities)
   }