Skip to content

Pass bulkPartialUpdate errors to taskStore.errors#198606

Merged
ersin-erdal merged 6 commits intoelastic:mainfrom
ersin-erdal:198428-pass-bulk-update-errors
Nov 6, 2024
Merged

Pass bulkPartialUpdate errors to taskStore.errors#198606
ersin-erdal merged 6 commits intoelastic:mainfrom
ersin-erdal:198428-pass-bulk-update-errors

Conversation

@ersin-erdal
Copy link
Contributor

@ersin-erdal ersin-erdal commented Oct 31, 2024

Resolves: #198428

@ersin-erdal ersin-erdal added Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor labels Oct 31, 2024
# Conflicts:
#	x-pack/plugins/task_manager/server/lib/create_managed_configuration.ts
Copy link
Contributor

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM, besides the comment I made about the optional message parameter

type,
}: {
statusCode: number;
message?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this should be optioonal. No idea what Error() does :-)

I think the potential undefined comes from the task_store usage, accessing item.update.error.reason, which is optional. Let's use a message like bulk update failed with unknown reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@ersin-erdal ersin-erdal marked this pull request as ready for review November 5, 2024 11:37
@ersin-erdal ersin-erdal requested a review from a team as a code owner November 5, 2024 11:37
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

Copy link
Contributor

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM, left a nit comment ...

}

export function getBulkUpdateStatusCode(error: Error | BulkUpdateError): number | undefined {
if (Boolean(error && error instanceof BulkUpdateError)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think this has the same result, with no cast :-)

  if (!(error instanceof BulkUpdateError)) return;
  return error.statusCode;

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

@ersin-erdal ersin-erdal merged commit 9f4d88e into elastic:main Nov 6, 2024
@ersin-erdal ersin-erdal deleted the 198428-pass-bulk-update-errors branch November 6, 2024 18:09
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11709378310

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 6, 2024
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

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 Nov 6, 2024
…99205)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Pass bulkPartialUpdate errors to taskStore.errors
(#198606)](#198606)

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

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

<!--BACKPORT [{"author":{"name":"Ersin
Erdal","email":"92688503+ersin-erdal@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-11-06T18:09:00Z","message":"Pass
bulkPartialUpdate errors to taskStore.errors (#198606)\n\nResolves:
#198428","sha":"9f4d88e4b5e491e53aa4cc0b2f6394f5a26e8d9c","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:ResponseOps","v9.0.0","backport:prev-minor"],"title":"Pass
bulkPartialUpdate errors to
taskStore.errors","number":198606,"url":"https://github.com/elastic/kibana/pull/198606","mergeCommit":{"message":"Pass
bulkPartialUpdate errors to taskStore.errors (#198606)\n\nResolves:
#198428","sha":"9f4d88e4b5e491e53aa4cc0b2f6394f5a26e8d9c"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198606","number":198606,"mergeCommit":{"message":"Pass
bulkPartialUpdate errors to taskStore.errors (#198606)\n\nResolves:
#198428","sha":"9f4d88e4b5e491e53aa4cc0b2f6394f5a26e8d9c"}}]}]
BACKPORT-->

Co-authored-by: Ersin Erdal <92688503+ersin-erdal@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v8.17.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ResponseOps][TaskManager] Errors returned by the taskStore.bulkPartialUpdate are not passed to taskStore.errors$

4 participants