Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9bf9e11
More robust error handling when uploading a file
ilyannn Aug 26, 2024
a46dac2
Clear the error and state on any changes to FileReader
ilyannn Aug 26, 2024
40cb9d1
Handle special Chrome behaviour
ilyannn Aug 26, 2024
dff63a0
Display longer message with onabort as well
ilyannn Aug 26, 2024
1711427
Merge branch 'main' into auto-import/better-errors
ilyannn Aug 26, 2024
56f878d
Use a different ID for the new string
ilyannn Aug 27, 2024
c1b77bc
Merge branch 'main' into auto-import/better-errors
elasticmachine Aug 27, 2024
eb0d7ef
Fix code duplication
ilyannn Aug 27, 2024
ed3535b
Merge branch 'auto-import/better-errors' of github.com:ilyannn/kibana…
ilyannn Aug 27, 2024
b49ca07
Extract a named function for handling an error
ilyannn Aug 27, 2024
571eb5a
Merge branch 'main' of github.com:elastic/kibana into auto-import/bet…
ilyannn Aug 31, 2024
ac196e8
Improve how the EMPTY error is handled
ilyannn Aug 31, 2024
97413f3
Merge branch 'main' into auto-import/better-errors
elasticmachine Aug 31, 2024
4cf9a49
Add a test for too large message
ilyannn Sep 2, 2024
5abbd5b
Remove unnecessary clear
ilyannn Sep 2, 2024
f6f3911
Add another test
ilyannn Sep 2, 2024
d0fee07
Improve documentation of parseLogsContent
ilyannn Sep 2, 2024
a86ae71
Explicitely use undefined
ilyannn Sep 2, 2024
6548743
Merge branch 'main' into auto-import/better-errors
elasticmachine Sep 2, 2024
a3fc3b6
Add a test case
ilyannn Sep 3, 2024
bc9a75c
Merge branch 'auto-import/better-errors' of github.com:ilyannn/kibana…
ilyannn Sep 3, 2024
dc7f6ea
Merge branch 'main' into auto-import/better-errors
elasticmachine Sep 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -305,4 +305,80 @@ describe('SampleLogsInput', () => {
});
});
});

describe('when the file is too large', () => {
const type = 'text/plain';
let jsonParseSpy: jest.SpyInstance;

beforeEach(async () => {
// Simulate large log content that would cause a RangeError
jsonParseSpy = jest.spyOn(JSON, 'parse').mockImplementation(() => {
throw new RangeError();
});

await changeFile(input, new File(['...'], 'test.json', { type }));
});

afterAll(() => {
// Restore the original implementation after all tests
jsonParseSpy.mockRestore();
Comment thread
semd marked this conversation as resolved.
});

it('should raise an appropriate error', () => {
expect(result.queryByText('This logs sample file is too large to parse')).toBeInTheDocument();
});
});

describe('when the file is neither a valid json nor ndjson', () => {
const plainTextFile = 'test message 1\ntest message 2';
const type = 'text/plain';

beforeEach(async () => {
await changeFile(input, new File([plainTextFile], 'test.txt', { type }));
});

it('should set the integrationSetting correctly', () => {
expect(mockActions.setIntegrationSettings).toBeCalledWith({
logSamples: plainTextFile.split('\n'),
samplesFormat: undefined,
});
});
});

describe('when the file reader fails', () => {
const mockedMessage = 'Mocked error';
let myFileReader: FileReader;
let fileReaderSpy: jest.SpyInstance;

beforeEach(async () => {
myFileReader = new FileReader();
fileReaderSpy = jest.spyOn(global, 'FileReader').mockImplementation(() => myFileReader);

// We need to mock the error property
Object.defineProperty(myFileReader, 'error', {
value: new Error(mockedMessage),
writable: false,
});

jest.spyOn(myFileReader, 'readAsText').mockImplementation(() => {
const errorEvent = new ProgressEvent('error');
myFileReader.dispatchEvent(errorEvent);
});

const file = new File([`...`], 'test.json', { type: 'application/json' });
act(() => {
fireEvent.change(input, { target: { files: [file] } });
});
});

afterEach(() => {
fileReaderSpy.mockRestore();
});

it('should set the error message accordingly', () => {
expect(
result.queryByText(`An error occurred when reading logs sample: ${mockedMessage}`)
).toBeInTheDocument();
});
});
});
Comment thread
semd marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,32 @@ export const parseJSONArray = (
return { errorNoArrayFound: true, entries: [], pathToEntries: [] };
};

interface ParseLogsErrorResult {
error: string;
}

interface ParseLogsSuccessResult {
// Format of the samples, if able to be determined.
samplesFormat?: SamplesFormat;
// The parsed log samples. If samplesFormat is (ND)JSON, these are JSON strings.
logSamples: string[];
}

type ParseLogsResult = ParseLogsErrorResult | ParseLogsSuccessResult;

/**
* Parse the logs sample file content (json or ndjson) and return the parsed logs sample
* Parse the logs sample file content and return the parsed logs sample.
*
* This function will return an error message if the file content is not valid, that is:
* - it is too large to parse (the memory required is 2-3x of the file size); or
* - it looks like a JSON format, but there is no array; or
* - it looks like (ND)JSON format, but the items are not JSON dictionaries.
*
* Otherwise it is guaranteed to parse and return (possibly empty) `logSamples` array.
* If the file content is (ND)JSON, it will additionally fill out the `samplesFormat`
* field with name 'json' or 'ndjson'; otherwise it will be undefined.
*/
const parseLogsContent = (
fileContent: string
): {
error?: string;
logSamples: string[];
samplesFormat?: SamplesFormat;
} => {
const parseLogsContent = (fileContent: string): ParseLogsResult => {
let parsedContent: unknown[];
let samplesFormat: SamplesFormat;

Expand All @@ -90,31 +106,37 @@ const parseLogsContent = (
samplesFormat = { name: 'ndjson', multiline: false };
}
} catch (parseNDJSONError) {
if (parseNDJSONError instanceof RangeError) {
return { error: i18n.LOGS_SAMPLE_ERROR.TOO_LARGE_TO_PARSE };
}
try {
const { entries, pathToEntries, errorNoArrayFound } = parseJSONArray(fileContent);
if (errorNoArrayFound) {
return { error: i18n.LOGS_SAMPLE_ERROR.NOT_ARRAY, logSamples: [] };
return { error: i18n.LOGS_SAMPLE_ERROR.NOT_ARRAY };
}
parsedContent = entries;
samplesFormat = { name: 'json', json_path: pathToEntries };
} catch (parseJSONError) {
if (parseJSONError instanceof RangeError) {
return { error: i18n.LOGS_SAMPLE_ERROR.TOO_LARGE_TO_PARSE };
}
try {
parsedContent = parseNDJSON(fileContent, true);
samplesFormat = { name: 'ndjson', multiline: true };
} catch (parseMultilineNDJSONError) {
if (parseMultilineNDJSONError instanceof RangeError) {
return { error: i18n.LOGS_SAMPLE_ERROR.TOO_LARGE_TO_PARSE };
}
return {
logSamples: fileContent.split('\n').filter((line) => line.trim() !== ''),
samplesFormat: undefined, // Signifies that the format is unknown.
};
}
}
}

if (parsedContent.length === 0) {
return { error: i18n.LOGS_SAMPLE_ERROR.EMPTY, logSamples: [] };
}

if (parsedContent.some((log) => !isPlainObject(log))) {
return { error: i18n.LOGS_SAMPLE_ERROR.NOT_OBJECT, logSamples: [] };
return { error: i18n.LOGS_SAMPLE_ERROR.NOT_OBJECT };
}

const logSamples = parsedContent.map((log) => JSON.stringify(log));
Expand All @@ -133,50 +155,84 @@ export const SampleLogsInput = React.memo<SampleLogsInputProps>(({ integrationSe

const onChangeLogsSample = useCallback(
(files: FileList | null) => {
const logsSampleFile = files?.[0];
if (logsSampleFile == null) {
setSampleFileError(undefined);
setIntegrationSettings({
...integrationSettings,
logSamples: undefined,
samplesFormat: undefined,
});
if (!files) {
return;
}

setSampleFileError(undefined);
setIntegrationSettings({
...integrationSettings,
logSamples: undefined,
samplesFormat: undefined,
});

const logsSampleFile = files[0];
const reader = new FileReader();

reader.onloadstart = function () {
setIsParsing(true);
};

reader.onloadend = function () {
setIsParsing(false);
};

reader.onload = function (e) {
const fileContent = e.target?.result as string | undefined; // We can safely cast to string since we call `readAsText` to load the file.

if (fileContent == null) {
return { error: i18n.LOGS_SAMPLE_ERROR.CAN_NOT_READ };
setSampleFileError(i18n.LOGS_SAMPLE_ERROR.CAN_NOT_READ);
return;
}
let samples;
const { error, logSamples, samplesFormat } = parseLogsContent(fileContent);
setIsParsing(false);
samples = logSamples;

if (error) {
setSampleFileError(error);
setIntegrationSettings({
...integrationSettings,
logSamples: undefined,
samplesFormat: undefined,
});

if (fileContent === '' && e.loaded > 100000) {
// V8-based browsers can't handle large files and return an empty string
// instead of an error; see https://stackoverflow.com/a/61316641
setSampleFileError(i18n.LOGS_SAMPLE_ERROR.TOO_LARGE_TO_PARSE);
return;
}

const result = parseLogsContent(fileContent);

if ('error' in result) {
setSampleFileError(result.error);
return;
}

const { logSamples: possiblyLargeLogSamples, samplesFormat } = result;

if (possiblyLargeLogSamples.length === 0) {
setSampleFileError(i18n.LOGS_SAMPLE_ERROR.EMPTY);
return;
}

if (samples.length > MaxLogsSampleRows) {
samples = samples.slice(0, MaxLogsSampleRows);
let logSamples;
if (possiblyLargeLogSamples.length > MaxLogsSampleRows) {
logSamples = possiblyLargeLogSamples.slice(0, MaxLogsSampleRows);
notifications?.toasts.addInfo(i18n.LOGS_SAMPLE_TRUNCATED(MaxLogsSampleRows));
} else {
logSamples = possiblyLargeLogSamples;
}

setIntegrationSettings({
...integrationSettings,
logSamples: samples,
logSamples,
samplesFormat,
});
};
setIsParsing(true);

const handleReaderError = function () {
const message = reader.error?.message;
if (message) {
setSampleFileError(i18n.LOGS_SAMPLE_ERROR.CAN_NOT_READ_WITH_REASON(message));
} else {
setSampleFileError(i18n.LOGS_SAMPLE_ERROR.CAN_NOT_READ);
}
};

reader.onerror = handleReaderError;
reader.onabort = handleReaderError;

reader.readAsText(logsSampleFile);
},
[integrationSettings, setIntegrationSettings, notifications?.toasts, setIsParsing]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,26 @@ export const LOGS_SAMPLE_ERROR = {
defaultMessage: 'Failed to read the logs sample file',
}
),
CAN_NOT_READ_WITH_REASON: (reason: string) =>
i18n.translate(
'xpack.integrationAssistant.step.dataStream.logsSample.errorCanNotReadWithReason',
{
values: { reason },
defaultMessage: 'An error occurred when reading logs sample: {reason}',
}
),
CAN_NOT_PARSE: i18n.translate(
'xpack.integrationAssistant.step.dataStream.logsSample.errorCanNotParse',
{
defaultMessage: 'Cannot parse the logs sample file as either a JSON or NDJSON file',
}
),
TOO_LARGE_TO_PARSE: i18n.translate(
'xpack.integrationAssistant.step.dataStream.logsSample.errorTooLargeToParse',
{
defaultMessage: 'This logs sample file is too large to parse',
}
),
NOT_ARRAY: i18n.translate('xpack.integrationAssistant.step.dataStream.logsSample.errorNotArray', {
defaultMessage: 'The logs sample file is not an array',
}),
Expand Down