Skip to content

Commit

Permalink
provide finer detail on action execution errors (elastic#52146)
Browse files Browse the repository at this point in the history
resolves elastic#52103
  • Loading branch information
pmuellr committed Dec 9, 2019
1 parent 80d35bd commit 2218ca5
Show file tree
Hide file tree
Showing 22 changed files with 157 additions and 194 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const actionTypeRegistryParams = {
beforeEach(() => jest.resetAllMocks());

const executor: ExecutorType = async options => {
return { status: 'ok' };
return { status: 'ok', actionId: options.actionId };
};

describe('register()', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const actionTypeRegistryParams = {
let actionsClient: ActionsClient;
let actionTypeRegistry: ActionTypeRegistry;
const executor: ExecutorType = async options => {
return { status: 'ok' };
return { status: 'ok', actionId: options.actionId };
};

beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,19 +174,17 @@ async function executor(
result = await sendEmail(logger, sendEmailOptions);
} catch (err) {
const message = i18n.translate('xpack.actions.builtin.email.errorSendingErrorMessage', {
defaultMessage: 'error in action "{actionId}" sending email: {errorMessage}',
values: {
actionId,
errorMessage: err.message,
},
defaultMessage: 'error sending email',
});
return {
status: 'error',
actionId,
message,
serviceMessage: err.message,
};
}

return { status: 'ok', data: result };
return { status: 'ok', data: result, actionId };
}

// utilities
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,11 @@ async function executor(

if (config.index == null && params.index == null) {
const message = i18n.translate('xpack.actions.builtin.esIndex.indexParamRequiredErrorMessage', {
defaultMessage: 'index param needs to be set because not set in config for action {actionId}',
values: {
actionId,
},
defaultMessage: 'index param needs to be set because not set in config for action',
});
return {
status: 'error',
actionId,
message,
};
}
Expand Down Expand Up @@ -101,17 +99,15 @@ async function executor(
result = await services.callCluster('bulk', bulkParams);
} catch (err) {
const message = i18n.translate('xpack.actions.builtin.esIndex.errorIndexingErrorMessage', {
defaultMessage: 'error in action "{actionId}" indexing data: {errorMessage}',
values: {
actionId,
errorMessage: err.message,
},
defaultMessage: 'error indexing documents',
});
return {
status: 'error',
actionId,
message,
serviceMessage: err.message,
};
}

return { status: 'ok', data: result };
return { status: 'ok', data: result, actionId };
}
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,12 @@ describe('execute()', () => {
}
`);
expect(actionResponse).toMatchInlineSnapshot(`
Object {
"data": "data-here",
"status": "ok",
}
`);
Object {
"actionId": "some-action-id",
"data": "data-here",
"status": "ok",
}
`);
});

test('should succeed with maximal valid params for trigger', async () => {
Expand Down Expand Up @@ -212,11 +213,12 @@ describe('execute()', () => {
}
`);
expect(actionResponse).toMatchInlineSnapshot(`
Object {
"data": "data-here",
"status": "ok",
}
`);
Object {
"actionId": "some-action-id",
"data": "data-here",
"status": "ok",
}
`);
});

test('should succeed with maximal valid params for acknowledge', async () => {
Expand Down Expand Up @@ -267,11 +269,12 @@ describe('execute()', () => {
}
`);
expect(actionResponse).toMatchInlineSnapshot(`
Object {
"data": "data-here",
"status": "ok",
}
`);
Object {
"actionId": "some-action-id",
"data": "data-here",
"status": "ok",
}
`);
});

test('should succeed with maximal valid params for resolve', async () => {
Expand Down Expand Up @@ -322,11 +325,12 @@ describe('execute()', () => {
}
`);
expect(actionResponse).toMatchInlineSnapshot(`
Object {
"data": "data-here",
"status": "ok",
}
`);
Object {
"actionId": "some-action-id",
"data": "data-here",
"status": "ok",
}
`);
});

test('should fail when sendPagerdury throws', async () => {
Expand All @@ -348,11 +352,13 @@ describe('execute()', () => {
};
const actionResponse = await actionType.executor(executorOptions);
expect(actionResponse).toMatchInlineSnapshot(`
Object {
"message": "error in pagerduty action \\"some-action-id\\" posting event: doing some testing",
"status": "error",
}
`);
Object {
"actionId": "some-action-id",
"message": "error posting pagerduty event",
"serviceMessage": "doing some testing",
"status": "error",
}
`);
});

test('should fail when sendPagerdury returns 429', async () => {
Expand All @@ -374,12 +380,13 @@ describe('execute()', () => {
};
const actionResponse = await actionType.executor(executorOptions);
expect(actionResponse).toMatchInlineSnapshot(`
Object {
"message": "error in pagerduty action \\"some-action-id\\" posting event: status 429, retry later",
"retry": true,
"status": "error",
}
`);
Object {
"actionId": "some-action-id",
"message": "error posting pagerduty event: http status 429, retry later",
"retry": true,
"status": "error",
}
`);
});

test('should fail when sendPagerdury returns 501', async () => {
Expand All @@ -401,12 +408,13 @@ describe('execute()', () => {
};
const actionResponse = await actionType.executor(executorOptions);
expect(actionResponse).toMatchInlineSnapshot(`
Object {
"message": "error in pagerduty action \\"some-action-id\\" posting event: status 501, retry later",
"retry": true,
"status": "error",
}
`);
Object {
"actionId": "some-action-id",
"message": "error posting pagerduty event: http status 501, retry later",
"retry": true,
"status": "error",
}
`);
});

test('should fail when sendPagerdury returns 418', async () => {
Expand All @@ -428,10 +436,11 @@ describe('execute()', () => {
};
const actionResponse = await actionType.executor(executorOptions);
expect(actionResponse).toMatchInlineSnapshot(`
Object {
"message": "error in pagerduty action \\"some-action-id\\" posting event: unexpected status 418",
"status": "error",
}
`);
Object {
"actionId": "some-action-id",
"message": "error posting pagerduty event: unexpected status 418",
"status": "error",
}
`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,14 @@ async function executor(
response = await postPagerduty({ apiUrl, data, headers, services });
} catch (err) {
const message = i18n.translate('xpack.actions.builtin.pagerduty.postingErrorMessage', {
defaultMessage: 'error in pagerduty action "{actionId}" posting event: {errorMessage}',
values: {
actionId,
errorMessage: err.message,
},
defaultMessage: 'error posting pagerduty event',
});
logger.warn(`error thrown posting pagerduty event: ${err.message}`);
return {
status: 'error',
actionId,
message,
serviceMessage: err.message,
};
}

Expand All @@ -141,38 +139,37 @@ async function executor(
if (response.status === 202) {
return {
status: 'ok',
actionId,
data: response.data,
};
}

if (response.status === 429 || response.status >= 500) {
const message = i18n.translate('xpack.actions.builtin.pagerduty.postingRetryErrorMessage', {
defaultMessage:
'error in pagerduty action "{actionId}" posting event: status {status}, retry later',
defaultMessage: 'error posting pagerduty event: http status {status}, retry later',
values: {
actionId,
status: response.status,
},
});

return {
status: 'error',
actionId,
message,
retry: true,
};
}

const message = i18n.translate('xpack.actions.builtin.pagerduty.postingUnexpectedErrorMessage', {
defaultMessage:
'error in pagerduty action "{actionId}" posting event: unexpected status {status}',
defaultMessage: 'error posting pagerduty event: unexpected status {status}',
values: {
actionId,
status: response.status,
},
});

return {
status: 'error',
actionId,
message,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,15 @@ async function executor(
logger[params.level](params.message);
} catch (err) {
const message = i18n.translate('xpack.actions.builtin.serverLog.errorLoggingErrorMessage', {
defaultMessage: 'error in action "{actionId}" logging message: {errorMessage}',
values: {
actionId,
errorMessage: err.message,
},
defaultMessage: 'error logging message',
});
return {
status: 'error',
message,
serviceMessage: err.message,
actionId,
};
}

return { status: 'ok' };
return { status: 'ok', actionId };
}
Loading

0 comments on commit 2218ca5

Please sign in to comment.