-
Notifications
You must be signed in to change notification settings - Fork 1
Update reporting functions to include a partial context in the error #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
7052e7e
feat: update report error and warning fn
xnanodax 9516ed0
update tests
xnanodax bc0b386
add transition to draft to active reporting options
xnanodax 64d9199
address code review comments
xnanodax 012ae70
update second arg to reporting function
xnanodax 2678be1
remove ts expect and switch to cast
xnanodax 44c4e05
feat: update report error and warning fn
xnanodax 1ee31c1
update tests
xnanodax 891cda6
add transition to draft to active reporting options
xnanodax 8e62288
address code review comments
xnanodax c8e0a25
update second arg to reporting function
xnanodax fd0870c
Merge branch 'report-error-fns' of github.com:zendesk/react-measure-t…
xnanodax 1a11e2d
fix: improvements to error reporting and warning
niieani 875eb00
test: partially update test
xnanodax e7a6ee8
test: update failing tests
xnanodax db335ea
Update src/v3/TraceManager.ts
xnanodax File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
|
@@ -929,8 +931,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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this was the best way we can address this time for the time being |
||
this as Trace<any, RelationSchemasT, any>, | ||
) | ||
} | ||
return this.input as ActiveTraceConfig< | ||
|
@@ -1071,6 +1077,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>, | ||
) | ||
} | ||
|
||
|
@@ -1221,21 +1229,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 = { | ||
|
@@ -1250,7 +1313,11 @@ export class Trace< | |
this.applyDefinitionModifications(inputAndDefinitionModifications) | ||
|
||
this.wasActivated = true | ||
this.stateMachine.emit('onMakeActive', undefined) | ||
|
||
if (isDraft) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. only drafts can switch the state to active |
||
// we might already be active in which case we would have issued a warning earlier in this method | ||
this.stateMachine.emit('onMakeActive', undefined) | ||
} | ||
} | ||
|
||
/** | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small clean up: replacing throw error with this.traceUtilities.reportErrorFn