Unify error reporting across Stack Management UIs

[cjcenizal]

Problems

Incompatible SectionError component

Index Management has its own SectionError component. This component is incompatible with the ES UI Shared SectionError component, because they each expect different types of error to be provided to them. ES UI Shared expects the error object to have an error property, whereas Index Management expects it to have statusText and attributes properties.

// ES UI Shared
const {
  cause, // wrapEsError() on the server adds a "cause" array
  message,
  error: errorString,
} = error;

// Index Management
const {
  cause: causeRoot, // wrapEsError() on the server adds a "cause" array
  message,
  statusText,
  attributes: { cause: causeAttributes } = {},
} = error;

statusText: From what I can tell, the statusText property is never actually provided on the error object by the server, so we might be able to ignore that.

attributes: The attributes property is provided in three API route handlers, all within the api/templates/ directory:

• register_update_route: https://github.com/elastic/kibana/blob/master/x-pack/plugins/index_management/server/routes/api/templates/register_update_route.ts#L54
• register_create_route: https://github.com/elastic/kibana/blob/master/x-pack/plugins/index_management/server/routes/api/templates/register_create_route.ts#L60
• register_simulate_route: https://github.com/elastic/kibana/blob/master/x-pack/plugins/index_management/server/routes/api/templates/register_simulate_route.ts

Cost: Aside from DX confusion (making UI migrations difficult), this impedes us from improving supportability by enhancing SectionError error reporting with any features added on the ES side (e.g. domain-specific error codes).

Resolution: We need to resolve this incompatibility and then migrate from Index Management's SectionError to the ES UI Shared SectionError.

Incompatibilities with ErrorToast and addError

The Toasts module's addError method accepts a global Error argument. This requires weird hacks to fulfill the interface's expectations.

The Toasts module defines its own RequestError type, which seems like it should be defined as part of the elasticsearch-specification repo, perhaps as the ErrorResponseBase type. If that's not possible, this type should be exported and we should consume it in ES UI Shared and elsewhere in our plugins.

Cost: DX confusion makes it challenging to consistently implement error reporting and error handling in a straightforward manner. These incompatibilities also impeded us from improving supportability by enhancing addError error reporting with any features added on the ES side (e.g. domain-specific error codes).

Resolution: We need to resolve these incompatibilities by refactoring addError to narrowly define the types of errors it can accept. These types of errors should be published in a centralized location and consumed by the relevant plugins.

Related issues

• Unify Elasticsearch error message processing/formatting in SearchSource to provided unified experience across Kibana (Unify Elasticsearch error message processing/formatting in SearchSource to provided unified experience across Kibana #62868)
• Provide Error handling guide for Kibana developers (Provide Error handling guide for Kibana developers #99568)
• [Metrics UI] Improve API error handling for ES queries ([Metrics UI] Improve API error handling for ES queries #96084)

[elasticmachine]

Pinging @elastic/kibana-stack-management (Team:Stack Management)

[cjcenizal]

CC @elastic/kibana-core

[pgayvallet]

Incompatibilities with ErrorToast and addError

We need to resolve these incompatibilities by refactoring addError to narrowly define the types of errors it can accept

We could, but the problem with TS error handling is that, by nature, you can't really be sure what your error's shape is, and you can't restraint a catch block to only specific error types, making any type of specialized-error-type-as-argument patterns potentially dangerous, as the caught error is any anyway, and would match anything.

So, even if we were to specialize the accepted input of toast.addError, it wouldn't be properly handled in most usages of the API:

type Toast = {
   addError: (error: SomeSpecificError | AnotherSpecificError) => ReturnType
}

try {
 explode()
} catch(e) {
  // TS is awesome, e is any.
  Toast.addError(e);
}

Also, this would introduce bad isolation, as core's showErrorDialog implementation would have to handle potentially consumer-specific error types, which we're trying to avoid.

So, if I agree that the hacks around the Toast handling of RequestError is an aberration, AFAIK this is a necessary evil to handle the most common case where the error caught was send by the server, and is 'fine' due to the fact that is a core handled(-ish) error.

If a consumer wants to render specific toast messages (and associated dialogs) for a specialized error type, it should be done via a custom component and using the toast.add({color: 'danger', text: myErrorMountPoint}) API instead.

[mshustov]

We could, but the problem with TS error handling is that, by nature, you can't really be sure what your error's shape is, and you can't restraint a catch block to only specific error types, making any type of specialized-error-type-as-argument patterns potentially dangerous, as the caught error is any anyway, and would match anything.

Starting from TS v4.4, the catch argument will be treated as unknown in strict mode. Considering that we track any usages count in the codebase, we might hope all the errors will be properly typed in the future.

Also, this would introduce bad isolation, as core's showErrorDialog implementation would have to handle potentially consumer-specific error types, which we're trying to avoid.

my overall 👍 However, I don't see anything bad if relax requirements for the input argument:

interface ErrorLike {
  message: string;
  stack?: string;
}

interface RequestError extends ErrorLike { ... }

[tsullivan]

I think this proposal should interest @elastic/kibana-app-services. The search services typically uses a "danger" toast rendered with a custom React component when showing an error. The way it is done uses a custom class that extends Error, and it is extremely hard to find the "important" part of the error. See #104466 (comment)