Add error return value from page.on handlers
#1495
Merged
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
This will make it simpler to read and maintain the page.on handler code.
When the context is closed, we should return an error to notify the user that not all handlers completed execution.
The most important thing to do is to handle the error and ensure that the page.on metric returns false so that the metric tag isn't update. This is being defensive, since the context has already closed and the metric emitter will not emit a metric when the context has closed. We're logging a debug log here since this information will not benefit the end user and when this error occurs they can't action on it. It could be useful for debugging though.
Mainly useful for debugging purposes, but otherwise this isn't really needed except to ensure we exit asap from the for loop when the context has ended and therefore the iteration has ended.
3 tasks
inancgumus
approved these changes
Oct 18, 2024
Yeah, definitely agree. |
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?
This adds an error return value from the
page.onhandlers.Why?
It's being more defensive to ensure that when an error occurs (due to the iteration ending) we exit early and don't try to process any other handlers, and to ensure that we don't use incorrect url name values when a
page,.on(metric)handler returns early.Checklist
Related PR(s)/Issue(s)
Relates to: #1493 (comment)