Skip to content
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

Handle missing VDU better in In QuickJS scanner plugin #5058

Merged
merged 1 commit into from
May 17, 2024

Conversation

nickva
Copy link
Contributor

@nickva nickva commented May 17, 2024

Previously we tried to run the VDU query prompt even if there was not VDU. That would have yielded a match anyway as both result in an error, but it added noise to the logs.

Handle the missing VDU like we handle the missing filters in filter_doc_validate.

@nickva nickva force-pushed the check-for-vdu-in-quickjs-scanner branch from 076c94b to 536807d Compare May 17, 2024 00:15
Copy link
Contributor

@jaydoane jaydoane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth adding a test for this?

@nickva
Copy link
Contributor Author

nickva commented May 17, 2024

Is it worth adding a test for this?

@jaydoane Good idea. Will add a few tests for VDU only, filter only, maps only etc. Will need to check if process proc errors were raised, we record those in couch_stats so I'll sample those at the end of the plugin run.

Previously we tried to run the VDU query prompt even if there was not VDU. That
would have yielded a match anyway as both result in an error, but it added
noise to the logs.

Handle the missing VDU like we handle the missing filters in
filter_doc_validate.
@nickva nickva force-pushed the check-for-vdu-in-quickjs-scanner branch from 536807d to 910fc26 Compare May 17, 2024 05:19
@nickva nickva merged commit 833e4c3 into main May 17, 2024
23 checks passed
@nickva nickva deleted the check-for-vdu-in-quickjs-scanner branch May 17, 2024 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants