Skip to content

Commit

Permalink
[ML] Reverting use of isPopulatedObject in error utils (elastic#159913)
Browse files Browse the repository at this point in the history
PR elastic#155372 moved our error utils
to a package and also made a few small code changes, one of which added
`isPopulatedObject` to the error object type guards.
`isPopulatedObject` uses `Object.keys` under the hood which cannot be
used to access the non-enumerable properties of an object, like Error's
`message`.


![image](https://github.com/elastic/kibana/assets/22172091/6a0269df-ca2a-494a-9364-8f35f2b52388)

This PR reverts all of these functions back to their original versions
which had existed in ML for a while without issue.

This was change had been causing error messages to not display
correctly.


![image](https://github.com/elastic/kibana/assets/22172091/1862f069-1626-4ac3-8961-dca016b91956)

vs


![image](https://github.com/elastic/kibana/assets/22172091/243143a5-0c8f-4365-a41d-7c1c09858ad8)
  • Loading branch information
jgowdyelastic authored Jun 19, 2023
1 parent f91ee9c commit 8ca1789
Showing 1 changed file with 14 additions and 19 deletions.
33 changes: 14 additions & 19 deletions x-pack/packages/ml/error_utils/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import type Boom from '@hapi/boom';
import * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';

import type { IHttpFetchError } from '@kbn/core-http-browser';
import { isPopulatedObject } from '@kbn/ml-is-populated-object';

/**
* Short hand type of estypes.ErrorCause.
Expand Down Expand Up @@ -144,53 +143,49 @@ export type ErrorType = MLHttpFetchError | EsErrorBody | Boom.Boom | string | un
/**
* Type guard to check if error is of type EsErrorBody
* @export
* @param {unknown} error
* @param {any} error
* @returns {error is EsErrorBody}
*/
export function isEsErrorBody(error: unknown): error is EsErrorBody {
return isPopulatedObject(error, ['error']) && isPopulatedObject(error.error, ['reason']);
export function isEsErrorBody(error: any): error is EsErrorBody {
return error && error.error?.reason !== undefined;
}

/**
* Type guard to check if error is a string.
* @export
* @param {unknown} error
* @param {any} error
* @returns {error is string}
*/
export function isErrorString(error: unknown): error is string {
export function isErrorString(error: any): error is string {
return typeof error === 'string';
}

/**
* Type guard to check if error is of type ErrorMessage.
* @export
* @param {unknown} error
* @param {any} error
* @returns {error is ErrorMessage}
*/
export function isErrorMessage(error: unknown): error is ErrorMessage {
return isPopulatedObject(error, ['message']) && typeof error.message === 'string';
export function isErrorMessage(error: any): error is ErrorMessage {
return error && error.message !== undefined && typeof error.message === 'string';
}

/**
* Type guard to check if error is of type MLResponseError.
* @export
* @param {unknown} error
* @param {any} error
* @returns {error is MLResponseError}
*/
export function isMLResponseError(error: unknown): error is MLResponseError {
return (
isPopulatedObject(error, ['body']) &&
isPopulatedObject(error.body, ['message']) &&
'message' in error.body
);
export function isMLResponseError(error: any): error is MLResponseError {
return typeof error.body === 'object' && 'message' in error.body;
}

/**
* Type guard to check if error is of type Boom.
* @export
* @param {unknown} error
* @param {any} error
* @returns {error is Boom.Boom}
*/
export function isBoomError(error: unknown): error is Boom.Boom {
return isPopulatedObject(error, ['isBoom']) && error.isBoom === true;
export function isBoomError(error: any): error is Boom.Boom {
return error?.isBoom === true;
}

0 comments on commit 8ca1789

Please sign in to comment.