Skip to content

[7.5] [Reporting/Screenshots] Do not fail the report if request is aborted (#52344)#54596

Merged
tsullivan merged 1 commit intoelastic:7.5from
tsullivan:backport/7.5/pr-52344
Jan 13, 2020
Merged

[7.5] [Reporting/Screenshots] Do not fail the report if request is aborted (#52344)#54596
tsullivan merged 1 commit intoelastic:7.5from
tsullivan:backport/7.5/pr-52344

Conversation

@tsullivan
Copy link
Copy Markdown
Member

@tsullivan tsullivan commented Jan 13, 2020

Closes #54464

Backports the following commits to 7.5:

…lastic#52344)

* [Reporting/Screenshots] Do not fail the report if request is aborted

* take pageRequestFailed out of pageExit observable
@tsullivan tsullivan added the backport This PR is a backport of another PR label Jan 13, 2020
const failure = req.failure && req.failure();
if (failure) {
this.logger.warning(
`Request to [${req.url()}] failed! [${failure.errorText}]. This error will be ignored.`
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This would make page errors (such as the newsfeed service being unavailable) to be logged as a warning instead of cause the reporting job to fail.

Copy link
Copy Markdown
Contributor

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

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

code change LGTM. I'll leave approving this change in behavior to someone else that can make this decision.

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR regression v7.5.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants