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

Handle content stream errors in report pre-deletion #173792

Merged
merged 6 commits into from Jan 2, 2024
Merged

Handle content stream errors in report pre-deletion #173792

merged 6 commits into from Jan 2, 2024

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Dec 20, 2023 •
edited by kibanamachine

Re-addresses #171363

The bug was still evident, especially when using network throttling to add slight lag to the request turnaround times.

This PR adds more handling of errors that could be thrown slightly prior to deleting the report document, when we try to clear all chunks of the report using the content stream.

Before
byugyhgaslisdtur-before.mp4
After
byugyhgaslisdtur-after.mp4

Checklist

  • Unit tests

@tsullivan
Copy link
Member Author

buildkite build this

@tsullivan tsullivan marked this pull request as ready for review December 21, 2023 20:21
@tsullivan tsullivan requested a review from a team as a code owner December 21, 2023 20:21
tsullivan
tsullivan commented Dec 21, 2023
View reviewed changes
x-pack/plugins/reporting/server/routes/internal/management/integration_tests/jobs.test.ts

describe('successful downloads', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

changes above this line are to just restructure the tests

tsullivan
tsullivan commented Dec 21, 2023
View reviewed changes
x-pack/plugins/reporting/server/routes/public/integration_tests/jobs.test.ts

describe('successful downloads', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

changes above this line are to just restructure the tests

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

rshen91
rshen91 approved these changes Jan 2, 2024
View reviewed changes
x-pack/plugins/reporting/server/routes/common/jobs/get_job_routes.ts
// passed to the `stream.end` callback from
// the _final method of the ContentStream.
// This event must be handled.
stream.on('error', (err) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

noice!!

@tsullivan tsullivan added backport:prev-minor Backport to the previous minor version (i.e. one version back from main) backport:prev-MAJOR Backport to the previous major branch (7.x) and all later branches still in development labels Jan 2, 2024
@tsullivan tsullivan merged commit dc813c3 into elastic:main Jan 2, 2024
24 checks passed
@tsullivan tsullivan mentioned this pull request Jan 2, 2024
Closed
tsullivan added a commit to tsullivan/kibana that referenced this pull request Jan 2, 2024
@tsullivan
Handle content stream errors in report pre-deletion (elastic#173792)
bf70e08 
Re-addresses elastic#171363

The bug was still evident, especially when using network throttling to
add slight lag to the request turnaround times.

This PR adds more handling of errors that could be thrown slightly prior
to deleting the report document, when we try to clear all chunks of the
report using the content stream.

<details>
<summary>Before</summary>

https://github.com/elastic/kibana/assets/908371/c27fe314-0f93-42b4-8076-99a1e30b8d2f

</details>

<details>
<summary>After</summary>

https://github.com/elastic/kibana/assets/908371/4c1f5edd-73f1-4ca4-a40a-f900ca5f9c78

</details>

### Checklist
- [x] Unit tests

(cherry picked from commit dc813c3)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 2, 2024
@tsullivan
Handle content stream errors in report pre-deletion (elastic#173792)
e03d70b 
Re-addresses elastic#171363

The bug was still evident, especially when using network throttling to
add slight lag to the request turnaround times.

This PR adds more handling of errors that could be thrown slightly prior
to deleting the report document, when we try to clear all chunks of the
report using the content stream.

<details>
<summary>Before</summary>

https://github.com/elastic/kibana/assets/908371/c27fe314-0f93-42b4-8076-99a1e30b8d2f

</details>

<details>
<summary>After</summary>

https://github.com/elastic/kibana/assets/908371/4c1f5edd-73f1-4ca4-a40a-f900ca5f9c78

</details>

### Checklist
- [x] Unit tests

(cherry picked from commit dc813c3)
@kibanamachine kibanamachine mentioned this pull request Jan 2, 2024
Merged
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
❌ 7.17 Backport failed because of merge conflicts

You might need to backport the following PRs to 7.17:
- chore(NA): bump node into v20 (#173461)
- Replace deprecated node-sass with sass #2 (#173942)
✅ 8.12

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 173792

Questions ?

Please refer to the Backport tool documentation

@tsullivan tsullivan mentioned this pull request Jan 2, 2024
Merged
tsullivan added a commit to tsullivan/kibana that referenced this pull request Jan 2, 2024
@tsullivan
Handle content stream errors in report pre-deletion (elastic#173792)
41fd648 
Re-addresses elastic#171363

The bug was still evident, especially when using network throttling to
add slight lag to the request turnaround times.

This PR adds more handling of errors that could be thrown slightly prior
to deleting the report document, when we try to clear all chunks of the
report using the content stream.

<details>
<summary>Before</summary>

https://github.com/elastic/kibana/assets/908371/c27fe314-0f93-42b4-8076-99a1e30b8d2f

</details>

<details>
<summary>After</summary>

https://github.com/elastic/kibana/assets/908371/4c1f5edd-73f1-4ca4-a40a-f900ca5f9c78

</details>

### Checklist
- [x] Unit tests

(cherry picked from commit dc813c3)

# Conflicts:
#	x-pack/plugins/reporting/server/routes/common/jobs/get_job_routes.ts
@tsullivan tsullivan mentioned this pull request Jan 2, 2024
Merged
@tsullivan
Copy link
Member Author

tsullivan commented Jan 2, 2024 •
edited

💚 All backports created successfully

Status Branch Result
✅ 8.12
✅ 8.11
✅ 7.17

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jan 3, 2024
@kibanamachine @tsullivan
[8.12] Handle content stream errors in report pre-deletion (#173792) (#…
73951f9 
…174139)

# Backport

This will backport the following commits from `main` to `8.12`:
- [Handle content stream errors in report pre-deletion
(#173792)](#173792)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Tim
Sullivan","email":"tsullivan@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-01-02T23:00:53Z","message":"Handle
content stream errors in report pre-deletion (#173792)\n\nRe-addresses
#171363 bug was still
evident, especially when using network throttling to\r\nadd slight lag
to the request turnaround times.\r\n\r\nThis PR adds more handling of
errors that could be thrown slightly prior\r\nto deleting the report
document, when we try to clear all chunks of the\r\nreport using the
content
stream.\r\n\r\n<details>\r\n<summary>Before</summary>\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/c27fe314-0f93-42b4-8076-99a1e30b8d2f\r\n\r\n\r\n</details>\r\n\r\n<details>\r\n<summary>After</summary>\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/4c1f5edd-73f1-4ca4-a40a-f900ca5f9c78\r\n\r\n\r\n</details>\r\n\r\n###
Checklist\r\n- [x] Unit
tests","sha":"dc813c351fe111c895e85a188372ad31625d8c8c","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","backport:prev-minor","backport:prev-MAJOR","v8.13.0"],"title":"Handle
content stream errors in report
pre-deletion","number":173792,"url":"#173792
content stream errors in report pre-deletion (#173792)\n\nRe-addresses
#171363 bug was still
evident, especially when using network throttling to\r\nadd slight lag
to the request turnaround times.\r\n\r\nThis PR adds more handling of
errors that could be thrown slightly prior\r\nto deleting the report
document, when we try to clear all chunks of the\r\nreport using the
content
stream.\r\n\r\n<details>\r\n<summary>Before</summary>\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/c27fe314-0f93-42b4-8076-99a1e30b8d2f\r\n\r\n\r\n</details>\r\n\r\n<details>\r\n<summary>After</summary>\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/4c1f5edd-73f1-4ca4-a40a-f900ca5f9c78\r\n\r\n\r\n</details>\r\n\r\n###
Checklist\r\n- [x] Unit
tests","sha":"dc813c351fe111c895e85a188372ad31625d8c8c"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.13.0","branchLabelMappingKey":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"#173792
content stream errors in report pre-deletion (#173792)\n\nRe-addresses
#171363 bug was still
evident, especially when using network throttling to\r\nadd slight lag
to the request turnaround times.\r\n\r\nThis PR adds more handling of
errors that could be thrown slightly prior\r\nto deleting the report
document, when we try to clear all chunks of the\r\nreport using the
content
stream.\r\n\r\n<details>\r\n<summary>Before</summary>\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/c27fe314-0f93-42b4-8076-99a1e30b8d2f\r\n\r\n\r\n</details>\r\n\r\n<details>\r\n<summary>After</summary>\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/4c1f5edd-73f1-4ca4-a40a-f900ca5f9c78\r\n\r\n\r\n</details>\r\n\r\n###
Checklist\r\n- [x] Unit
tests","sha":"dc813c351fe111c895e85a188372ad31625d8c8c"}}]}] BACKPORT-->

Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
This was referenced Jan 3, 2024
Merged
Merged
jloleysens added a commit that referenced this pull request Jan 4, 2024
@jloleysens
Merge branch 'main' into versioned-router-and-oas
912d763 
* main: (4129 commits)
  [Logs Explorer] Change the default link for "Discover" in the serverless nav (#173420)
  [Fleet] fix unhandled error in agent details when components are missing (#174152)
  [Obs UX] Unskip transaction duration alerts test (#174069)
  [Fleet] Fix keep policies up to date after package install (#174093)
  [Profiling] Stack traces embeddable (#173905)
  [main] Sync bundled packages with Package Storage (#174115)
  [SLO Form] Refactor to use kibana data view component (#173513)
  [Obs UX] Unskip APM Service Inventory Journey (#173510)
  [Obs UX] Unskip preview_chart_error_count test (#173624)
  [api-docs] 2024-01-03 Daily api_docs build (#174142)
  Update babel runtime helpers (#171745)
  Handle content stream errors in report pre-deletion (#173792)
  [Cloud Posture] [Quick wins] Enable edit DataView on the Misconfigurations Findings Table (#173870)
  [ftr] abort retry on invalid webdriver session (#174092)
  Upgrade openai to 4.24.1 (#173934)
  chore(NA): bump node into v20 (#173461)
  [Security Solution][Endpoint] Fix index name pattern in SentinelOne dev. script (#174105)
  fix versions.json
  [Obs AI Assistant] Add guardrails (#174060)
  [ML] Transforms: Refactor validators and add unit tests. (#173736)
  ...
tsullivan added a commit that referenced this pull request Jan 4, 2024
@tsullivan
[8.11] Handle content stream errors in report pre-deletion (#173792) (#…
f5436fc 
…174140)

# Backport

This will backport the following commits from `main` to `8.11`:
- [Handle content stream errors in report pre-deletion
(#173792)](#173792)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Tim
Sullivan","email":"tsullivan@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-01-02T23:00:53Z","message":"Handle
content stream errors in report pre-deletion (#173792)\n\nRe-addresses
#171363 bug was still
evident, especially when using network throttling to\r\nadd slight lag
to the request turnaround times.\r\n\r\nThis PR adds more handling of
errors that could be thrown slightly prior\r\nto deleting the report
document, when we try to clear all chunks of the\r\nreport using the
content
stream.\r\n\r\n<details>\r\n<summary>Before</summary>\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/c27fe314-0f93-42b4-8076-99a1e30b8d2f\r\n\r\n\r\n</details>\r\n\r\n<details>\r\n<summary>After</summary>\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/4c1f5edd-73f1-4ca4-a40a-f900ca5f9c78\r\n\r\n\r\n</details>\r\n\r\n###
Checklist\r\n- [x] Unit
tests","sha":"dc813c351fe111c895e85a188372ad31625d8c8c","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","backport:prev-minor","backport:prev-MAJOR","v8.12.0","v8.13.0","v8.11.4"],"number":173792,"url":"#173792
content stream errors in report pre-deletion (#173792)\n\nRe-addresses
#171363 bug was still
evident, especially when using network throttling to\r\nadd slight lag
to the request turnaround times.\r\n\r\nThis PR adds more handling of
errors that could be thrown slightly prior\r\nto deleting the report
document, when we try to clear all chunks of the\r\nreport using the
content
stream.\r\n\r\n<details>\r\n<summary>Before</summary>\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/c27fe314-0f93-42b4-8076-99a1e30b8d2f\r\n\r\n\r\n</details>\r\n\r\n<details>\r\n<summary>After</summary>\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/4c1f5edd-73f1-4ca4-a40a-f900ca5f9c78\r\n\r\n\r\n</details>\r\n\r\n###
Checklist\r\n- [x] Unit
tests","sha":"dc813c351fe111c895e85a188372ad31625d8c8c"}},"sourceBranch":"main","suggestedTargetBranches":["8.12","8.11"],"targetPullRequestStates":[{"branch":"8.12","label":"v8.12.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.13.0","labelRegex":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"#173792
content stream errors in report pre-deletion (#173792)\n\nRe-addresses
#171363 bug was still
evident, especially when using network throttling to\r\nadd slight lag
to the request turnaround times.\r\n\r\nThis PR adds more handling of
errors that could be thrown slightly prior\r\nto deleting the report
document, when we try to clear all chunks of the\r\nreport using the
content
stream.\r\n\r\n<details>\r\n<summary>Before</summary>\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/c27fe314-0f93-42b4-8076-99a1e30b8d2f\r\n\r\n\r\n</details>\r\n\r\n<details>\r\n<summary>After</summary>\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/4c1f5edd-73f1-4ca4-a40a-f900ca5f9c78\r\n\r\n\r\n</details>\r\n\r\n###
Checklist\r\n- [x] Unit
tests","sha":"dc813c351fe111c895e85a188372ad31625d8c8c"}},{"branch":"8.11","label":"v8.11.4","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
tsullivan added a commit that referenced this pull request Jan 4, 2024
@tsullivan
[7.17] Handle content stream errors in report pre-deletion (#173792) (#…
eb687c4 
…174141)

# Backport

This will backport the following commits from `main` to `7.17`:
- [Handle content stream errors in report pre-deletion
(#173792)](#173792)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Tim
Sullivan","email":"tsullivan@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-01-02T23:00:53Z","message":"Handle
content stream errors in report pre-deletion (#173792)\n\nRe-addresses
#171363 bug was still
evident, especially when using network throttling to\r\nadd slight lag
to the request turnaround times.\r\n\r\nThis PR adds more handling of
errors that could be thrown slightly prior\r\nto deleting the report
document, when we try to clear all chunks of the\r\nreport using the
content
stream.\r\n\r\n<details>\r\n<summary>Before</summary>\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/c27fe314-0f93-42b4-8076-99a1e30b8d2f\r\n\r\n\r\n</details>\r\n\r\n<details>\r\n<summary>After</summary>\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/4c1f5edd-73f1-4ca4-a40a-f900ca5f9c78\r\n\r\n\r\n</details>\r\n\r\n###
Checklist\r\n- [x] Unit
tests","sha":"dc813c351fe111c895e85a188372ad31625d8c8c","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","backport:prev-minor","backport:prev-MAJOR","v8.12.0","v8.13.0","v8.11.4"],"number":173792,"url":"#173792
content stream errors in report pre-deletion (#173792)\n\nRe-addresses
#171363 bug was still
evident, especially when using network throttling to\r\nadd slight lag
to the request turnaround times.\r\n\r\nThis PR adds more handling of
errors that could be thrown slightly prior\r\nto deleting the report
document, when we try to clear all chunks of the\r\nreport using the
content
stream.\r\n\r\n<details>\r\n<summary>Before</summary>\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/c27fe314-0f93-42b4-8076-99a1e30b8d2f\r\n\r\n\r\n</details>\r\n\r\n<details>\r\n<summary>After</summary>\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/4c1f5edd-73f1-4ca4-a40a-f900ca5f9c78\r\n\r\n\r\n</details>\r\n\r\n###
Checklist\r\n- [x] Unit
tests","sha":"dc813c351fe111c895e85a188372ad31625d8c8c"}},"sourceBranch":"main","suggestedTargetBranches":["8.12","8.11"],"targetPullRequestStates":[{"branch":"8.12","label":"v8.12.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.13.0","labelRegex":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"#173792
content stream errors in report pre-deletion (#173792)\n\nRe-addresses
#171363 bug was still
evident, especially when using network throttling to\r\nadd slight lag
to the request turnaround times.\r\n\r\nThis PR adds more handling of
errors that could be thrown slightly prior\r\nto deleting the report
document, when we try to clear all chunks of the\r\nreport using the
content
stream.\r\n\r\n<details>\r\n<summary>Before</summary>\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/c27fe314-0f93-42b4-8076-99a1e30b8d2f\r\n\r\n\r\n</details>\r\n\r\n<details>\r\n<summary>After</summary>\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/908371/4c1f5edd-73f1-4ca4-a40a-f900ca5f9c78\r\n\r\n\r\n</details>\r\n\r\n###
Checklist\r\n- [x] Unit
tests","sha":"dc813c351fe111c895e85a188372ad31625d8c8c"}},{"branch":"8.11","label":"v8.11.4","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
@tsullivan tsullivan deleted the reporting/delete-route-safety branch April 30, 2024 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rshen91 rshen91 rshen91 approved these changes

Assignees
No one assigned
Labels
backport:prev-MAJOR Backport to the previous major branch (7.x) and all later branches still in development backport:prev-minor Backport to the previous minor version (i.e. one version back from main) release_note:fix v7.17.17 v8.11.4 v8.12.0 v8.13.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tsullivan @kibana-ci @kibanamachine @rshen91