Starting a new TTVC measurement will now cancel out the previous measurement, if it is still active #85
Conversation
This file contains 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
kierrrrra
requested review from
hongrich,
ajhyndman,
umairnadeem and
Jonathan1600
kierrrrra
commented
Apr 11, 2024
| */ | ||
| export const cancel = (eventType?: string) => calculator?.cancel(eventType); | ||
| export const cancel = (e?: Event) => calculator?.cancel(e); |
There was a problem hiding this comment.
I know that this is changing the API, so will be a breaking change.
kierrrrra
commented
Apr 11, 2024
| expect(entries[1].detail.navigationType).toBe('script'); | ||
|
|
||
| // We expect the interrupted measurement to be ended before the start of the new measurement | ||
| expect(errors[0].end).toBeLessThanOrEqual(entries[1].start + FUDGE); |
There was a problem hiding this comment.
This is the crux of the fix, really
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What is this
While working on tighter integration of TTVC measurements of "soft" navigations, I have ran into an issue. While it is admittedly an edge case it still warrants a fix: when a soft navigation occurs before the measurement has had a chance to complete, the interrupted navigation will not be reported entirely correctly.
The diagram below might help understand the issue:
In this situation:
Along the way, I have also addressed a few edge case scenarios:
NEW_MEASUREMENTcancellation reason, which is slightly distinct fromNEW_NAVIGATION.NEW_NAVIGATIONmight mean a navigation to an entirely different site; WhileNEW_MEASUREMENTrefers to a new measurement on an existing site.Testing
spa6, to test for this scenario.yarn:expressand hit thehttp://localhost:3000/test/spa6#test case. Wait for a hard navigation to complete. Click one of the links, and immediately click another link. You will see that a cancellation of the first soft navigation TTVC measurement will occur before the second soft navigation.