Skip to content

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
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@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 that referenced this pull request Jul 2, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@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