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

Improve brimcap analyze error handling #1850

Merged
merged 1 commit into from
Sep 28, 2021
Merged

Conversation

mason-fish
Copy link
Contributor

fixes #1820

When we load a pool with the brimcap plugin, we pass each specific load operation the output stream from a forked brimcap analyze process. If the analyze process fails the load operation will fail too, but we have not been handling the load and/or analyze errors properly. This PR aims to fix that: now if load fails, we will check for an analyze error and report that if it exists, otherwise we report the load error.

I think this is also a bit cleaner to use a single error flag, analyzeErr that is a dedicated error bucket for the analyze operation, as opposed to a general error bucket for the analyze, load, and index operations.

Signed-off-by: Mason Fish mason@brimsecurity.com

Signed-off-by: Mason Fish <mason@brimsecurity.com>
@mason-fish mason-fish requested a review from a team September 27, 2021 22:04
Copy link
Member

@jameskerr jameskerr left a comment

Choose a reason for hiding this comment

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

Got it, I understand the issue. Looks like this will fix it.

@mason-fish mason-fish merged commit 47e6f16 into main Sep 28, 2021
@mason-fish mason-fish deleted the 1820-brimcap_plugin_errors branch September 28, 2021 17:38
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.

Errors from custom Brimcap config not surfaced in Brim app
2 participants