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

[Ingest Manager] Define custom errors by extending Error #69966

Merged
merged 3 commits into from
Jun 29, 2020

Conversation

jfsiii
Copy link
Contributor

@jfsiii jfsiii commented Jun 25, 2020

Summary

Create Error types using extends. e.g. define new types with

class SomeNewError extends Error {}
// or
class SomeNewError extends IngestManagerError {}

and create with

new SomeNewError('a message')

Why this way?

@jfsiii
Copy link
Contributor Author

jfsiii commented Jun 26, 2020

@elasticmachine merge upstream

export enum IngestManagerErrorType {
RegistryError,
}
export class RegistryError extends IngestManagerError {}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose to extend IngestManagerError here so that it gets the benefits of the stack trace naming from https://github.com/elastic/kibana/pull/69966/files#diff-305aab707fa2746fa91c6bec9e78c0b7R11 and remain an IngestManagerError

However, even if it did extend Error we could make a type like

type IngestManagerErrorType = IngestManagerError | RegistryError

This is where my mind is heading now. Files will define/export Errors where they are used (or perhaps lifted to common or server), and then those can be imported and combined into other types

@jfsiii jfsiii marked this pull request as ready for review June 26, 2020 20:43
@jfsiii jfsiii requested a review from a team June 26, 2020 20:43
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jun 26, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@jfsiii jfsiii added release_note:skip Skip the PR/issue when compiling release notes v7.9.0 v8.0.0 labels Jun 26, 2020
@jfsiii jfsiii self-assigned this Jun 26, 2020
@jfsiii
Copy link
Contributor Author

jfsiii commented Jun 29, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 It's nice to make things more "standard"

@jfsiii
Copy link
Contributor Author

jfsiii commented Jun 29, 2020

I'm going to merge this so I can use this pattern in some other places, and because it's not difficult to roll back, but I'm happy to revisit this if anyone wants.

@jfsiii jfsiii merged commit 7db95a1 into elastic:master Jun 29, 2020
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jun 30, 2020
…ata-streams

* 'master' of github.com:elastic/kibana: (50 commits)
  [Logs UI] [Alerting] "Group by" functionality (elastic#68250)
  [Discover] Deangularize Skip to bottom button (elastic#69811)
  Implement recursive plugin discovery (elastic#68811)
  Use ts-expect-error in platform code (elastic#69883)
  [SIEM][Detection Engine][Lists] Moves getQueryFilter to common folder for use by both front and backend
  [Ingest Manager][SECURITY SOLUTION] adjust config reassign link and add roundtrip to Reassignment flow (elastic#70208)
  [Security][Lists] Add API functions and react hooks for value list APIs (elastic#69603)
  [ILM] Fix bug when clearing priority field (elastic#70154)
  [Platform][Security] Updates cluster_manager ignorePaths to include security scripts (elastic#70139)
  [IngestManager] Allow to filter agent by packages (elastic#69731)
  [code coverage] exclude folders: test_helpers, tests_bundle (elastic#70199)
  [Metrics UI] UX improvements for saved views (elastic#69910)
  [APM] docs: unique transaction troubleshooting (elastic#69831)
  Cross cluster search functional test with minimun privileges assigned to the test_user (elastic#70007)
  [Maps] choropleth layer wizard (elastic#69699)
  Make custom errors by extending Error (elastic#69966)
  [Ingest Manager] Support updated package output structure (elastic#69864)
  Resolver test coverage (elastic#70246)
  Async Discover search test (elastic#64388)
  [ui-shared-deps] include styled-components (elastic#69322)
  ...

# Conflicts:
#	x-pack/plugins/snapshot_restore/server/types.ts
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 30, 2020
…bana into alerting/consumer-based-rbac

* 'alerting/consumer-based-rbac' of github.com:gmmorris/kibana: (49 commits)
  [Discover] Deangularize Skip to bottom button (elastic#69811)
  Implement recursive plugin discovery (elastic#68811)
  Use ts-expect-error in platform code (elastic#69883)
  [SIEM][Detection Engine][Lists] Moves getQueryFilter to common folder for use by both front and backend
  [Ingest Manager][SECURITY SOLUTION] adjust config reassign link and add roundtrip to Reassignment flow (elastic#70208)
  [Security][Lists] Add API functions and react hooks for value list APIs (elastic#69603)
  [ILM] Fix bug when clearing priority field (elastic#70154)
  [Platform][Security] Updates cluster_manager ignorePaths to include security scripts (elastic#70139)
  [IngestManager] Allow to filter agent by packages (elastic#69731)
  [code coverage] exclude folders: test_helpers, tests_bundle (elastic#70199)
  [Metrics UI] UX improvements for saved views (elastic#69910)
  [APM] docs: unique transaction troubleshooting (elastic#69831)
  Cross cluster search functional test with minimun privileges assigned to the test_user (elastic#70007)
  [Maps] choropleth layer wizard (elastic#69699)
  Make custom errors by extending Error (elastic#69966)
  [Ingest Manager] Support updated package output structure (elastic#69864)
  Resolver test coverage (elastic#70246)
  Async Discover search test (elastic#64388)
  [ui-shared-deps] include styled-components (elastic#69322)
  SECURITY-ENDPOINT: add host properties (elastic#70238)
  ...
Bamieh pushed a commit to Bamieh/kibana that referenced this pull request Jul 1, 2020
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 69966 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 1, 2020
jfsiii pushed a commit to jfsiii/kibana that referenced this pull request Jul 1, 2020
jfsiii pushed a commit that referenced this pull request Jul 2, 2020
Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 2, 2020
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:Fleet Team label for Observability Data Collection Fleet team v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants