Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fleet] catching only mapper errors #167044

Merged
merged 15 commits into from
Sep 26, 2023
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { safeLoad } from 'js-yaml';
import { loggerMock } from '@kbn/logging-mocks';
import { elasticsearchServiceMock } from '@kbn/core/server/mocks';

import { errors } from '@elastic/elasticsearch';

import { createAppContextStartContractMock } from '../../../../mocks';
import { appContextService } from '../../..';
import type { RegistryDataStream } from '../../../../types';
Expand Down Expand Up @@ -1261,5 +1263,65 @@ describe('EPM template', () => {
},
});
});
it('should rollover on expected error', async () => {
const esClient = elasticsearchServiceMock.createElasticsearchClient();
esClient.indices.getDataStream.mockResponse({
data_streams: [{ name: 'test.prefix1-default' }],
} as any);
esClient.indices.simulateTemplate.mockImplementation(() => {
throw new errors.ResponseError({
statusCode: 400,
body: {
error: {
type: 'illegal_argument_exception',
},
},
} as any);
});
const logger = loggerMock.create();
await updateCurrentWriteIndices(esClient, logger, [
{
templateName: 'test',
indexTemplate: {
index_patterns: ['test.*-*'],
template: {
settings: { index: {} },
mappings: { properties: {} },
},
} as any,
},
]);

expect(esClient.indices.rollover).toHaveBeenCalled();
});
it('should not rollover on unexpected error', async () => {
const esClient = elasticsearchServiceMock.createElasticsearchClient();
esClient.indices.getDataStream.mockResponse({
data_streams: [{ name: 'test.prefix1-default' }],
} as any);
esClient.indices.simulateTemplate.mockImplementation(() => {
throw new Error();
});
const logger = loggerMock.create();
try {
await updateCurrentWriteIndices(esClient, logger, [
{
templateName: 'test',
indexTemplate: {
index_patterns: ['test.*-*'],
template: {
settings: { index: {} },
mappings: { properties: {} },
},
} as any,
},
]);
fail('expected updateCurrentWriteIndices to throw error');
} catch (err) {
// noop
}

expect(esClient.indices.rollover).not.toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import type {
MappingTypeMapping,
} from '@elastic/elasticsearch/lib/api/typesWithBodyKey';

import { isResponseError } from '@kbn/es-errors';

import type { Field, Fields } from '../../fields/field';
import type {
RegistryDataStream,
Expand Down Expand Up @@ -795,10 +797,18 @@ const updateExistingDataStream = async ({

// if update fails, rollover data stream and bail out
} catch (err) {
logger.info(`Mappings update for ${dataStreamName} failed due to ${err}`);
logger.info(`Triggering a rollover for ${dataStreamName}`);
await rolloverDataStream(dataStreamName, esClient);
return;
if (
isResponseError(err) &&
err.statusCode === 400 &&
err.body?.error?.type === 'illegal_argument_exception'
) {
logger.info(`Mappings update for ${dataStreamName} failed due to ${err}`);
logger.info(`Triggering a rollover for ${dataStreamName}`);
await rolloverDataStream(dataStreamName, esClient);
return;
}
logger.error(`Mappings update for ${dataStreamName} failed due to unexpected error: ${err}`);
throw err;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concern here is that this might surface existing errors that were ignored before, but I think it's better to see the errors early.

}

// Trigger a rollover if the index mode or source type has changed
Expand Down