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.

Sign up for GitHub

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

Jump to bottom

[bfetch] Improve error handling #123455

Merged
merged 2 commits into from Jan 20, 2022
Merged

[bfetch] Improve error handling #123455

merged 2 commits into from Jan 20, 2022

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Jan 20, 2022 •
edited

Summary

Fixes error where bfetch didn't handle the response as an error when proxy streamed an error response. For example 502 Bad Gateway from azure.

This led to confusing errors like this #122664 where application code tried to parse an error response (usually plain html text) as base64 string or as json string

Note: this pr doesn't fix the #122664, it just fixed error handling that hides underlying network error

How to test

I added mock response from proxy here

return response.custom({
            statusCode: 502,
            bypassErrorFormat: true,
            headers: {
              'Content-Type': 'text/html',
            },
            body: `<html>
<head><title>502 Bad Gateway</title></head>
<body>
<center><h1>502 Bad Gateway</h1></center>
<hr><center>Microsoft-Azure-Application-Gateway/v2</center>
</body>
</html>
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->`,
          });

Before the fix

Screen Shot 2022-01-20 at 13 33 54

After the fix

Screen Shot 2022-01-20 at 13 34 50

Release Notes

Improve bfetch error handling

@Dosant Dosant marked this pull request as ready for review January 20, 2022 12:32
@Dosant Dosant requested a review from a team as a code owner January 20, 2022 12:32
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServicesSv)

@botelastic botelastic bot added the Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) label Jan 20, 2022
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
bfetch 7.4KB 7.4KB +24.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@Dosant Dosant merged commit 460f89a into elastic:main Jan 20, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jan 20, 2022
ogupte pushed a commit to ogupte/kibana that referenced this pull request Jan 28, 2022
@Dosant @ogupte
[bfetch] Improve error handling (elastic#123455)
c555d50
@Dosant Dosant mentioned this pull request Jan 28, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vadimkibana vadimkibana vadimkibana approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:Batching/Streaming Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) release_note:fix v8.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Dosant @elasticmachine @kibana-ci @vadimkibana @kibanamachine