-
Notifications
You must be signed in to change notification settings - Fork 291
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
Add evaluation error reporting for expressions #4342
Conversation
Reports runtime evaluation error (like NPE) into snapshots in evaluationErrors attribute.
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.
Looks good. I think we need to test more dedup scenarios but it could be it's own PR
List<ProbeDetails> additionalProbes = probe.additionalProbes; | ||
if (!additionalProbes.isEmpty()) { | ||
for (ProbeDetails additionalProbe : additionalProbes) { | ||
if (executeScript(additionalProbe.getScript(), capture, additionalProbe.id)) { | ||
capturingProbeIds.add(additionalProbe.id); | ||
} else if (capture.areEvalErrors()) { |
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.
maybe there is a bug here. if we have multiple probes and the first probe generate errors. so capture.areEvalErrors() will return true even if executeScript for the other probe return false without any errors.
We might need to clear errors before execute each script. Moreover, not sure if the errors for those probes are getting recorded
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.
Do we have tests for de-dup probes with different conditions working/failing?
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.
yeah I see. I will fix this in another PR. dup probes are still a special case, and not common right now.
What Does This Do
Reports runtime evaluation error (like NPE) into snapshots in
evaluationErrors
attribute.Motivation
Additional Notes