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 error management #4145

Closed
3 of 6 tasks
Machi3mfl opened this issue May 13, 2022 · 10 comments · Fixed by #4163, #5381 or #5382
Closed
3 of 6 tasks

Improve error management #4145

Machi3mfl opened this issue May 13, 2022 · 10 comments · Fixed by #4163, #5381 or #5382
Assignees

Comments

@Machi3mfl
Copy link
Member

Machi3mfl commented May 13, 2022

Wazuh Elastic Rev Security
4.x 7.x 4xxx Basic, ODFE, Xpack
Browser
Chrome, Firefox, Safari, etc

Description

On this issue, we will analyze all the error treatments.
We need to find all the cases where we catch errors and decide how to throw them to other application methods.

In this issue, we want to improve the error management in our app.
We need to normalize and improve the treatment of errors.

Steps to find the issue scope:

Related issue

@Machi3mfl
Copy link
Member Author

Machi3mfl commented May 16, 2022

Steps

  • 1. Tracking all error throw errors that don't return an error instance
  • 2. Searching those methods usage (previous step)
  • 3. Analyzing possible solutions, best error treatment
  • 4. Implement solution
  • 5. Test solutions
# File Unit Test Files  Status
1 /public/react-services/wz-api-check.js Added
2 /public/react-services/wz-request.ts Added
3 /public/react-services/generic-request.js Added
4 /public/react-services/saved-objects.js Added
5 /public/components/health-check/services/check-api.service.ts Added
6 /public/controllers/settings/settings.js Added

Actual services relationship

graph TD;
    check-api.service-->generic-request;
    check-api.service-->wz-api-check;
    saved-objects-->generic-request;
    wz-request-->axios
    generic-request-->axios
    wz-api-check-->axios
Loading

@Machi3mfl
Copy link
Member Author

Machi3mfl commented May 17, 2022

1. wz-api-check: public/react-services/wz-api-check.js

checkStored method

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/react-services/wz-api-check.js#L50-L61

Use trace

checkApiService

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/components/health-check/services/check-api.service.ts#L63-L71

componentDidMount

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/components/wz-menu/wz-menu.js#L93-L124

request

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/react-services/generic-request.js#L92-L114

genericReq

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/react-services/generic-request.js#L92-L114

checkApi method

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/react-services/wz-api-check.js#L89-L99

Use trace

trySetDefault

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/components/health-check/services/check-api.service.ts#L20-L48

ChangeAPI

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/components/wz-menu/wz-menu.js#L348-L382

onInit

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/controllers/settings/settings.js#L70-L102

tryToSetDefault

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/services/resolves/settings-wizard.js#L125-L149

@Machi3mfl
Copy link
Member Author

Machi3mfl commented May 17, 2022

2. wz-request: public/react-services/wz-request.ts

genericReq method

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/react-services/wz-request.ts#L62-L99

Use trace

componentDidMount

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/components/add-modules-data/sample-data.tsx#L80-L105

addSampleData

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/components/add-modules-data/sample-data.tsx#L146-L189

removeSampleData

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/components/add-modules-data/sample-data.tsx#L192-L228

editKey

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/components/settings/configuration/utils/configuration-handler.js#L21-L32

SampleDataWarning

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/components/visualize/components/sample-data-warning.js#L24-L40

checkCurrentSecurityPlatform

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/controllers/management/components/management/configuration/utils/wz-fetch.js#L511-L522

ReportingHandler

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/controllers/management/components/management/reporting/utils/reporting-handler.js#L20-L44

existStatisticsIndices

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/controllers/management/components/management/statistics/statistics-overview.js#L266-L283

startVis2Png

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/react-services/reporting.js#L116-L144

startConfigReport

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/react-services/reporting.js#L168-L198

login

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/react-services/wz-authentication.ts#L40-L53

csvReq method

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/react-services/wz-request.ts#L149-L153

Use trace

exportCsv

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/react-services/wz-csv.js#L23-L30

apiReq method

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/react-services/wz-request.ts#L118-L132

Use trace

apiReq is the most used app method, we need to check its use careful

@Machi3mfl
Copy link
Member Author

Machi3mfl commented May 17, 2022

3. generic-request: /public/react-services/generic-request.js

request method

https://github.com/wazuh/wazuh-kibana-app/blob/aa1a8e41a4337610889dd0332c63a1214ce91582/public/react-services/generic-request.js#L92-L115

Use trace

request method (GenericRequest.request) is used on 17 files and 40 times.

@Machi3mfl
Copy link
Member Author

Machi3mfl commented May 24, 2022

Applied solution:

  • Created ErrorFactory

Flow with Error Factory Implementation

graph TD;
 generic-request-->ThrowError
 wz-request-->ThrowError
 wz-api-check-->ThrowError
 ThrowError-->ErrorFactory
 ErrorFactory-->Error
 Error-->ErrorOrchestrator
 ErrorOrchestrator-->UI
 ErrorOrchestrator-->Browser-Console
 ErrorOrchestrator-->Logs
Loading

Iteration steps to cover all services and cases

  1. Use ErrorFactory createError in services
  2. Add unit tests

Base Services

  • generic-request
  • wz-request
  • wz-api-check

Services

  • check-api-service
  • saved-objects

@Machi3mfl
Copy link
Member Author

Machi3mfl commented Jun 2, 2022

This PR also resolves issue #4030 by implementing the use of the new Error Factory feature on root services

  • Added SettingsController tests to check the controller catches an Error instance instead of a string.

Issue screenshot

image

Solution screenshot

Screen.Recording.2022-06-02.at.14.05.42.mov

For testing

  • Restarting the service and going to Wazuh > Settings > API Configuration.
  • Click on the error toast "See full error" button and check if the error detail is shown.
  • Check the navigator console.

@Machi3mfl Machi3mfl linked a pull request Jun 2, 2022 that will close this issue
@gdiazlo gdiazlo moved this to Triage in Release 4.4.0 Jun 8, 2022
@AlexRuiz7 AlexRuiz7 changed the title [Feature] global error management on failed requests Global error management on failed requests Jun 24, 2022
@Machi3mfl
Copy link
Member Author

Machi3mfl commented Jun 27, 2022

Flow with Error Factory Implementation

graph TD;
 generic-request-->ThrowError
 wz-request-->ThrowError
 wz-api-check-->ThrowError
 ThrowError-->ErrorFactory
 ErrorFactory-->Error
 Error-->ErrorOrchestrator
 ErrorOrchestrator-->UI
 ErrorOrchestrator-->Browser-Console
 ErrorOrchestrator-->Logs
Loading

Advantages

  • Centralize the error management for the errors generated by the services
  • Catch all errors generated and guarantee to throw the correct error format for showing at UI
  • Add the capability to identify and create specific errors for all known error cases

@Machi3mfl
Copy link
Member Author

Machi3mfl commented Jun 27, 2022

Next steps

  • Identify error types received by the error factory
  • Create Errors classes for specific errors
  • Implement error treatment by each error type

Generic Error Types

EvalError

Creates an instance representing an error that occurs regarding the global function eval().

RangeError

Creates an instance representing an error that occurs when a numeric variable or parameter is outside its valid range.

ReferenceError

Creates an instance representing an error that occurs when de-referencing an invalid reference.

SyntaxError

Creates an instance representing a syntax error.

TypeError

Creates an instance representing an error that occurs when a variable or parameter is not of a valid type.

URIError

Creates an instance representing an error that occurs when encodeURI() or decodeURI() are passed invalid parameters.

AggregateError

Creates an instance representing several errors wrapped in a single error when multiple errors need to be reported by an operation, for example by Promise.any().

InternalError Non-Standard

Creates an instance representing an error that occurs when an internal error in the JavaScript engine is thrown. E.g. "too much recursion".

Error sources

  • Wazuh API errors
  • Elasticsearch errors
  • Operational errors (development)
  • Axios errors

@AlexRuiz7 AlexRuiz7 moved this from Triage to In Review in Release 4.4.0 Jul 18, 2022
@Machi3mfl Machi3mfl moved this from In Review to In Progress in Release 4.4.0 Aug 30, 2022
@vikman90 vikman90 added this to the Release 4.4.0 milestone Aug 31, 2022
@gdiazlo gdiazlo removed this from Release 4.4.0 Sep 1, 2022
@gdiazlo gdiazlo removed this from the Release 4.4.0 milestone Sep 5, 2022
@snaow snaow added this to the Release 4.5.0 milestone Nov 16, 2022
@snaow snaow removed this from the Release 4.5.0 milestone Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment