Skip to content

Commit

Permalink
[Ingest Pipelines] Error messages (#70167) (#70567)
Browse files Browse the repository at this point in the history
* improved error messages

* traverse recursive error struct

* add check for object with keys

* update button position and copy

* size adjustments

* Refactor i18n texts and change wording

Also added missing translation and refactored maximum errors in
collapsed state to external constant

* use io-ts, add CIT and unit tests

* refactor error utilities to separate file

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
jloleysens and elasticmachine authored Jul 2, 2020
1 parent 289652d commit 561fccc
Show file tree
Hide file tree
Showing 14 changed files with 473 additions and 42 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

export const nestedProcessorsErrorFixture = {
attributes: {
error: {
root_cause: [
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
suppressed: [
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
},
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
suppressed: [
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'csv',
},
],
},
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
},
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
},
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
},
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
},
],
},
],
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
suppressed: [
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
},
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
suppressed: [
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'csv',
},
],
},
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
},
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
},
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
},
{
type: 'parse_exception',
reason: '[field] required property is missing',
property_name: 'field',
processor_type: 'circle',
},
],
},
status: 400,
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ export type PipelineFormTestSubjects =
| 'submitButton'
| 'pageTitle'
| 'savePipelineError'
| 'savePipelineError.showErrorsButton'
| 'savePipelineError.hideErrorsButton'
| 'pipelineForm'
| 'versionToggle'
| 'versionField'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { act } from 'react-dom/test-utils';
import { setupEnvironment, pageHelpers, nextTick } from './helpers';
import { PipelinesCreateTestBed } from './helpers/pipelines_create.helpers';

import { nestedProcessorsErrorFixture } from './fixtures';

const { setup } = pageHelpers.pipelinesCreate;

jest.mock('@elastic/eui', () => {
Expand Down Expand Up @@ -163,6 +165,25 @@ describe('<PipelinesCreate />', () => {
expect(exists('savePipelineError')).toBe(true);
expect(find('savePipelineError').text()).toContain(error.message);
});

test('displays nested pipeline errors as a flat list', async () => {
const { actions, find, exists, waitFor } = testBed;
httpRequestsMockHelpers.setCreatePipelineResponse(undefined, {
body: nestedProcessorsErrorFixture,
});

await act(async () => {
actions.clickSubmitButton();
await waitFor('savePipelineError');
});

expect(exists('savePipelineError')).toBe(true);
expect(exists('savePipelineError.showErrorsButton')).toBe(true);
find('savePipelineError.showErrorsButton').simulate('click');
expect(exists('savePipelineError.hideErrorsButton')).toBe(true);
expect(exists('savePipelineError.showErrorsButton')).toBe(false);
expect(find('savePipelineError').find('li').length).toBe(8);
});
});

describe('test pipeline', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,18 @@ import { EuiButton, EuiButtonEmpty, EuiFlexGroup, EuiFlexItem, EuiSpacer } from
import { useForm, Form, FormConfig } from '../../../shared_imports';
import { Pipeline } from '../../../../common/types';

import { PipelineRequestFlyout } from './pipeline_request_flyout';
import { PipelineTestFlyout } from './pipeline_test_flyout';
import { PipelineFormFields } from './pipeline_form_fields';
import { PipelineFormError } from './pipeline_form_error';
import { pipelineFormSchema } from './schema';
import {
OnUpdateHandlerArg,
OnUpdateHandler,
SerializeResult,
} from '../pipeline_processors_editor';

import { PipelineRequestFlyout } from './pipeline_request_flyout';
import { PipelineTestFlyout } from './pipeline_test_flyout';
import { PipelineFormFields } from './pipeline_form_fields';
import { PipelineFormError } from './pipeline_form_error';
import { pipelineFormSchema } from './schema';

export interface PipelineFormProps {
onSave: (pipeline: Pipeline) => void;
onCancel: () => void;
Expand Down Expand Up @@ -116,7 +117,7 @@ export const PipelineForm: React.FunctionComponent<PipelineFormProps> = ({
error={form.getErrors()}
>
{/* Request error */}
{saveError && <PipelineFormError errorMessage={saveError.message} />}
{saveError && <PipelineFormError error={saveError} />}

{/* All form fields */}
<PipelineFormFields
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { toKnownError } from './error_utils';
import { nestedProcessorsErrorFixture } from '../../../../../__jest__/client_integration/fixtures';

describe('toKnownError', () => {
test('undefined, null, numbers, arrays and bad objects', () => {
expect(toKnownError(undefined)).toEqual({ errors: [{ reason: 'An unknown error occurred.' }] });
expect(toKnownError(null)).toEqual({ errors: [{ reason: 'An unknown error occurred.' }] });
expect(toKnownError(123)).toEqual({ errors: [{ reason: 'An unknown error occurred.' }] });
expect(toKnownError([])).toEqual({ errors: [{ reason: 'An unknown error occurred.' }] });
expect(toKnownError({})).toEqual({ errors: [{ reason: 'An unknown error occurred.' }] });
expect(toKnownError({ attributes: {} })).toEqual({
errors: [{ reason: 'An unknown error occurred.' }],
});
});

test('non-processors errors', () => {
expect(toKnownError(new Error('my error'))).toEqual({ errors: [{ reason: 'my error' }] });
expect(toKnownError({ message: 'my error' })).toEqual({ errors: [{ reason: 'my error' }] });
});

test('processors errors', () => {
expect(toKnownError(nestedProcessorsErrorFixture)).toMatchInlineSnapshot(`
Object {
"errors": Array [
Object {
"processorType": "circle",
"reason": "[field] required property is missing",
},
Object {
"processorType": "circle",
"reason": "[field] required property is missing",
},
Object {
"processorType": "circle",
"reason": "[field] required property is missing",
},
Object {
"processorType": "csv",
"reason": "[field] required property is missing",
},
Object {
"processorType": "circle",
"reason": "[field] required property is missing",
},
Object {
"processorType": "circle",
"reason": "[field] required property is missing",
},
Object {
"processorType": "circle",
"reason": "[field] required property is missing",
},
Object {
"processorType": "circle",
"reason": "[field] required property is missing",
},
],
}
`);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import * as t from 'io-ts';
import { flow } from 'fp-ts/lib/function';
import { isRight } from 'fp-ts/lib/Either';

import { i18nTexts } from './i18n_texts';

export interface PipelineError {
reason: string;
processorType?: string;
}
interface PipelineErrors {
errors: PipelineError[];
}

interface ErrorNode {
reason: string;
processor_type?: string;
suppressed?: ErrorNode[];
}

// This is a runtime type (RT) for an error node which is a recursive type
const errorNodeRT = t.recursion<ErrorNode>('ErrorNode', (ErrorNode) =>
t.intersection([
t.interface({
reason: t.string,
}),
t.partial({
processor_type: t.string,
suppressed: t.array(ErrorNode),
}),
])
);

// This is a runtime type for the attributes object we expect to receive from the server
// for processor errors
const errorAttributesObjectRT = t.interface({
attributes: t.interface({
error: t.interface({
root_cause: t.array(errorNodeRT),
}),
}),
});

const isProcessorsError = flow(errorAttributesObjectRT.decode, isRight);

type ErrorAttributesObject = t.TypeOf<typeof errorAttributesObjectRT>;

const flattenErrorsTree = (node: ErrorNode): PipelineError[] => {
const result: PipelineError[] = [];
const recurse = (_node: ErrorNode) => {
result.push({ reason: _node.reason, processorType: _node.processor_type });
if (_node.suppressed && Array.isArray(_node.suppressed)) {
_node.suppressed.forEach(recurse);
}
};
recurse(node);
return result;
};

export const toKnownError = (error: unknown): PipelineErrors => {
if (typeof error === 'object' && error != null && isProcessorsError(error)) {
const errorAttributes = error as ErrorAttributesObject;
const rootCause = errorAttributes.attributes.error.root_cause[0];
return { errors: flattenErrorsTree(rootCause) };
}

if (typeof error === 'string') {
return { errors: [{ reason: error }] };
}

if (
error instanceof Error ||
(typeof error === 'object' && error != null && (error as any).message)
) {
return { errors: [{ reason: (error as any).message }] };
}

return { errors: [{ reason: i18nTexts.errors.unknownError }] };
};
Loading

0 comments on commit 561fccc

Please sign in to comment.