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

TypeError resolved #4362

Merged
merged 7 commits into from
Aug 5, 2022
Merged

TypeError resolved #4362

merged 7 commits into from
Aug 5, 2022

Conversation

chantal-kelm
Copy link
Member

@chantal-kelm chantal-kelm commented Jul 28, 2022

Research

In public/plugin.ts a Get request is done with a Kibana resource core.http.get(/api/check-wazuh).

That resource fails in Chrome and Firefox browsers.

The Kibana core get resource is set up to catch the error with an HttpFetchError class that is a child of the Node Error class. Which is compatible only with browsers with V8 engine.
For this reason in Chrome it shows the error in the console as a debug and in Firefox it shows it in the console as an unrecognized error.

In the http_fetch_errors.ts and globals.d.ts files, kibana mentions V8 compatibility and that it will only work in browsers that use it. Attached screenshots
https://github.com/v8/v8/wiki/Stack%20Trace%20API#customizing-stack-traces

Screenshot from 2022-08-03 07-10-11
Screenshot from 2022-08-03 07-11-00

Proposed solution

Change the Get request that was made with a Kibana core.http.get(/api/check-wazuh) resource to the WzRequest.genericReq resource and it no longer fails, also add a test capture to public/plugin.ts that wraps the request and in case of failure, the error is detected when the browser does not work with the V8 engine.

Test

  • Click on the hamburger menu
  • Click on dashboard
  • Click on home
  • Click on Wazuh
  • Check that the console does not show: TypeError: NetworkError when attempting to fetch resource
videofirefox.mp4

Copy link
Member

@AlexRuiz7 AlexRuiz7 left a comment

Choose a reason for hiding this comment

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

This PR does not solve the problem. Take into account that is error might be a symptom of something else, like the App not being properly mounted / unmounted, not an isolated error.

Also, be thorough on the explanation on the cause of the problems and the work done to solve it.

In this case, incompatibilities with the V8 engine on Chrome and Firefox browsers are pointed as the cause of the error, but not a single reference supporting this theory is provided.

public/plugin.ts Outdated Show resolved Hide resolved
public/plugin.ts Outdated Show resolved Hide resolved
public/plugin.ts Outdated Show resolved Hide resolved
Copy link
Member

@Desvelao Desvelao left a comment

Choose a reason for hiding this comment

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

issue: the response variable doesn't have the endpoint response directly when using WzRequest.genericReq service, so it will not work as expected.

@AlexRuiz7 AlexRuiz7 marked this pull request as draft August 5, 2022 08:44
@AlexRuiz7 AlexRuiz7 marked this pull request as ready for review August 5, 2022 12:39
@chantal-kelm
Copy link
Member Author

chantal-kelm commented Aug 5, 2022

Fixed access to the isWazuhDisabled property and tested that it works by creating a user with a disabled role.

Screenshot from 2022-08-05 10-16-55
Screenshot from 2022-08-05 10-23-50

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@AlexRuiz7 AlexRuiz7 left a comment

Choose a reason for hiding this comment

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

  • Code review: ✔️
  • Test: ✔️

Note: check the changelog!

Copy link
Member

@Machi3mfl Machi3mfl left a comment

Choose a reason for hiding this comment

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

CR ✅
Test ✅

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2022

Jest Test Coverage % values
Statements 6.84% ( 2493 / 36463 )
Branches 2.72% ( 768 / 28219 )
Functions 4.58% ( 408 / 8914 )
Lines 6.92% ( 2413 / 34888 )

@AlexRuiz7 AlexRuiz7 merged commit 44a43cb into 4.3-7.10 Aug 5, 2022
@AlexRuiz7 AlexRuiz7 deleted the Fix/4346-TypeError branch August 5, 2022 16:05
@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2022

The backport to 4.3-7.16 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-4.3-7.16 4.3-7.16
# Navigate to the new working tree
cd .worktrees/backport-4.3-7.16
# Create a new branch
git switch --create backport-4362-to-4.3-7.16
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 44a43cbe28cf92824da60627b38ff50800474e07
# Push it to GitHub
git push --set-upstream origin backport-4362-to-4.3-7.16
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-4.3-7.16

Then, create a pull request where the base branch is 4.3-7.16 and the compare/head branch is backport-4362-to-4.3-7.16.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2022

The backport to 4.3-1.2-wzd failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-4.3-1.2-wzd 4.3-1.2-wzd
# Navigate to the new working tree
cd .worktrees/backport-4.3-1.2-wzd
# Create a new branch
git switch --create backport-4362-to-4.3-1.2-wzd
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 44a43cbe28cf92824da60627b38ff50800474e07
# Push it to GitHub
git push --set-upstream origin backport-4362-to-4.3-1.2-wzd
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-4.3-1.2-wzd

Then, create a pull request where the base branch is 4.3-1.2-wzd and the compare/head branch is backport-4362-to-4.3-1.2-wzd.

@chantal-kelm chantal-kelm restored the Fix/4346-TypeError branch August 5, 2022 16:38
chantal-kelm added a commit that referenced this pull request Aug 5, 2022
* Solve TypeError

* Add CHANGELOG

* Change request service

* Fix request's response structure

(cherry picked from commit 44a43cb)
chantal-kelm added a commit that referenced this pull request Aug 5, 2022
* Solve TypeError

* Add CHANGELOG

* Change request service

* Fix request's response structure

(cherry picked from commit 44a43cb)
chantal-kelm added a commit that referenced this pull request Aug 5, 2022
* Solve TypeError

* Add CHANGELOG

* Change request service

* Fix request's response structure

(cherry picked from commit 44a43cb)
chantal-kelm added a commit that referenced this pull request Aug 5, 2022
* Solve TypeError

* Add CHANGELOG

* Change request service

* Fix request's response structure

(cherry picked from commit 44a43cb)
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.

TypeError in the browser's developer console
4 participants