Skip to content

Commit c670dab

Browse files
authored
Revert "[Alerting] Handle when an Alerting Task fails due to its Alert object being deleted mid flight (#63093) (#63569)"
This reverts commit 3cdf259.
1 parent 3cdf259 commit c670dab

File tree

12 files changed

+21
-240
lines changed

12 files changed

+21
-240
lines changed

x-pack/plugins/alerting/server/alerts_client.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import {
3333
import { EncryptedSavedObjectsPluginStart } from '../../../plugins/encrypted_saved_objects/server';
3434
import { TaskManagerStartContract } from '../../../plugins/task_manager/server';
3535
import { taskInstanceToAlertTaskInstance } from './task_runner/alert_task_instance';
36-
import { deleteTaskIfItExists } from './lib/delete_task_if_it_exists';
3736

3837
type NormalizedAlertAction = Omit<AlertAction, 'actionTypeId'>;
3938
export type CreateAPIKeyResult =
@@ -264,7 +263,7 @@ export class AlertsClient {
264263
const removeResult = await this.savedObjectsClient.delete('alert', id);
265264

266265
await Promise.all([
267-
taskIdToRemove ? deleteTaskIfItExists(this.taskManager, taskIdToRemove) : null,
266+
taskIdToRemove ? this.taskManager.remove(taskIdToRemove) : null,
268267
apiKeyToInvalidate ? this.invalidateApiKey({ apiKey: apiKeyToInvalidate }) : null,
269268
]);
270269

@@ -506,9 +505,7 @@ export class AlertsClient {
506505
);
507506

508507
await Promise.all([
509-
attributes.scheduledTaskId
510-
? deleteTaskIfItExists(this.taskManager, attributes.scheduledTaskId)
511-
: null,
508+
attributes.scheduledTaskId ? this.taskManager.remove(attributes.scheduledTaskId) : null,
512509
apiKeyToInvalidate ? this.invalidateApiKey({ apiKey: apiKeyToInvalidate }) : null,
513510
]);
514511
}

x-pack/plugins/alerting/server/lib/delete_task_if_it_exists.test.ts

Lines changed: 0 additions & 44 deletions
This file was deleted.

x-pack/plugins/alerting/server/lib/delete_task_if_it_exists.ts

Lines changed: 0 additions & 17 deletions
This file was deleted.

x-pack/plugins/alerting/server/lib/is_alert_not_found_error.test.ts

Lines changed: 0 additions & 31 deletions
This file was deleted.

x-pack/plugins/alerting/server/lib/is_alert_not_found_error.ts

Lines changed: 0 additions & 11 deletions
This file was deleted.

x-pack/plugins/alerting/server/task_runner/task_runner.test.ts

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import { encryptedSavedObjectsMock } from '../../../../plugins/encrypted_saved_o
1414
import { savedObjectsClientMock, loggingServiceMock } from '../../../../../src/core/server/mocks';
1515
import { PluginStartContract as ActionsPluginStart } from '../../../actions/server';
1616
import { actionsMock } from '../../../actions/server/mocks';
17-
import { SavedObjectsErrorHelpers } from '../../../../../src/core/server';
1817

1918
const alertType = {
2019
id: 'test',
@@ -502,36 +501,4 @@ describe('Task Runner', () => {
502501
}
503502
`);
504503
});
505-
506-
test('avoids rescheduling a failed Alert Task Runner when it throws due to failing to fetch the alert', async () => {
507-
savedObjectsClient.get.mockImplementation(() => {
508-
throw SavedObjectsErrorHelpers.createGenericNotFoundError('task', '1');
509-
});
510-
511-
const taskRunner = new TaskRunner(
512-
alertType,
513-
mockedTaskInstance,
514-
taskRunnerFactoryInitializerParams
515-
);
516-
517-
encryptedSavedObjectsPlugin.getDecryptedAsInternalUser.mockResolvedValueOnce({
518-
id: '1',
519-
type: 'alert',
520-
attributes: {
521-
apiKey: Buffer.from('123:abc').toString('base64'),
522-
},
523-
references: [],
524-
});
525-
526-
const runnerResult = await taskRunner.run();
527-
528-
expect(runnerResult).toMatchInlineSnapshot(`
529-
Object {
530-
"runAt": undefined,
531-
"state": Object {
532-
"previousStartedAt": 1970-01-01T00:00:00.000Z,
533-
},
534-
}
535-
`);
536-
});
537504
});

x-pack/plugins/alerting/server/task_runner/task_runner.ts

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,12 @@ import {
2424
import { promiseResult, map, Resultable, asOk, asErr, resolveErr } from '../lib/result_type';
2525
import { taskInstanceToAlertTaskInstance } from './alert_task_instance';
2626
import { AlertInstances } from '../alert_instance/alert_instance';
27-
import { isAlertSavedObjectNotFoundError } from '../lib/is_alert_not_found_error';
2827

2928
const FALLBACK_RETRY_INTERVAL: IntervalSchedule = { interval: '5m' };
3029

3130
interface AlertTaskRunResult {
3231
state: AlertTaskState;
33-
runAt: Date | undefined;
32+
runAt: Date;
3433
}
3534

3635
interface AlertTaskInstance extends ConcreteTaskInstance {
@@ -294,29 +293,22 @@ export class TaskRunner {
294293
};
295294
},
296295
(err: Error) => {
297-
const message = `Executing Alert "${alertId}" has resulted in Error: ${err.message}`;
298-
if (isAlertSavedObjectNotFoundError(err, alertId)) {
299-
this.logger.debug(message);
300-
} else {
301-
this.logger.error(message);
302-
}
296+
this.logger.error(`Executing Alert "${alertId}" has resulted in Error: ${err.message}`);
303297
return {
304298
...originalState,
305299
previousStartedAt,
306300
};
307301
}
308302
),
309-
runAt: resolveErr<Date | undefined, Error>(runAt, err => {
310-
return isAlertSavedObjectNotFoundError(err, alertId)
311-
? undefined
312-
: getNextRunAt(
313-
new Date(),
314-
// if we fail at this point we wish to recover but don't have access to the Alert's
315-
// attributes, so we'll use a default interval to prevent the underlying task from
316-
// falling into a failed state
317-
FALLBACK_RETRY_INTERVAL
318-
);
319-
}),
303+
runAt: resolveErr<Date, Error>(runAt, () =>
304+
getNextRunAt(
305+
new Date(),
306+
// if we fail at this point we wish to recover but don't have access to the Alert's
307+
// attributes, so we'll use a default interval to prevent the underlying task from
308+
// falling into a failed state
309+
FALLBACK_RETRY_INTERVAL
310+
)
311+
),
320312
};
321313
}
322314
}

x-pack/plugins/oss_telemetry/server/lib/tasks/visualizations/task_runner.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7+
import moment from 'moment';
78
import {
89
getMockCallWithInternal,
910
getMockConfig,
@@ -12,7 +13,6 @@ import {
1213
} from '../../../test_utils';
1314
import { visualizationsTaskRunner } from './task_runner';
1415
import { TaskInstance } from '../../../../../task_manager/server';
15-
import { getNextMidnight } from '../../get_next_midnight';
1616

1717
describe('visualizationsTaskRunner', () => {
1818
let mockTaskInstance: TaskInstance;
@@ -41,6 +41,12 @@ describe('visualizationsTaskRunner', () => {
4141
});
4242

4343
test('Summarizes visualization response data', async () => {
44+
const getNextMidnight = () =>
45+
moment()
46+
.add(1, 'days')
47+
.startOf('day')
48+
.toDate();
49+
4450
const runner = visualizationsTaskRunner(mockTaskInstance, getMockConfig(), getMockEs());
4551
const result = await runner();
4652

x-pack/plugins/task_manager/server/lib/is_task_not_found_error.test.ts

Lines changed: 0 additions & 31 deletions
This file was deleted.

x-pack/plugins/task_manager/server/lib/is_task_not_found_error.ts

Lines changed: 0 additions & 11 deletions
This file was deleted.

0 commit comments

Comments
 (0)