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 the error handler on the configuration save #4116

Merged
merged 5 commits into from
May 3, 2022

Conversation

AlexRuiz7
Copy link
Member

Summary

Closes #4071

This PR improves the error handler when the configuration save fails.

This adds better messages that will ease the debugging if the error happens again.

manager_save

To test

  • Use this branch.
  • Apply this patch: force_save_error.txt
    git apply force_save_error.txt
  • Save the configuration.
  • An error toast MUST be shown, with the following text: 'This error has been forced. It is fine.'

@AlexRuiz7 AlexRuiz7 self-assigned this Apr 28, 2022
@AlexRuiz7 AlexRuiz7 linked an issue Apr 28, 2022 that may be closed by this pull request
@mauceballos mauceballos self-requested a review April 29, 2022 12:06
@mauceballos
Copy link
Contributor

Testing on kibana v7.10.2 OK

cluster-3

message: errorMessage || error,
title: `${error.name}: Mitre alerts could not be fetched`,
message: errorMessage || error,
title: "The configuration couldn't be saved.",
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: The file is saved, but it could be some validation errors. The message could be confusing.

@Desvelao
Copy link
Member

Desvelao commented May 3, 2022

review
Test
I tested the PR putting a not supported configuration tag (alerts_log2) as:

<ossec_config>
  <global>
    <jsonout_output>yes</jsonout_output>
    <alerts_log>yes</alerts_log>
    <alerts_log2 realtime2="yes" >no</alerts_log2>
   <!-- Other settings -->

and the toast is displayed, but the error is a string, so the conditional block is not modifying the error message. This behavior could be changed due to the chages in the PR #4119. I merged the last changes and it seems to be working.

image

@Desvelao Desvelao merged commit d1ece91 into 4.3-7.10 May 3, 2022
@Desvelao Desvelao deleted the fix/configuration-save-error branch May 3, 2022 09:14
@Desvelao Desvelao added the UI/UX Generic label for things related to the font-end side label May 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2022

Jest Test Coverage % values
Statements 4.05% ( 1478 / 36532 )
Branches 1.62% ( 461 / 28490 )
Functions 2.99% ( 267 / 8930 )
Lines 4.09% ( 1430 / 34928 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI/UX Generic label for things related to the font-end side
Projects
None yet
Development

Successfully merging this pull request may close these issues.

An error occurs when the configuration is changed for the first time
3 participants