Skip to content

Commit fcd2dc6

Browse files
cnasikascnasikas
committed
Use isStepReady
1 parent 9ed5eff commit fcd2dc6

8 files changed

Lines changed: 79 additions & 31 deletions

File tree

‎x-pack/platform/plugins/shared/alerting_v2/server/lib/rule_executor/steps/create_alert_events_step.test.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,28 +81,28 @@ describe('CreateAlertEventsStep', () => {
8181
});
8282
});
8383

84-
it('throws when rule is missing from state', async () => {
84+
it('halts with state_not_ready when rule is missing from state', async () => {
8585
const { loggerService } = createLoggerService();
8686
const { storageService } = createStorageService();
8787

8888
const step = new CreateAlertEventsStep(loggerService, storageService);
8989
const state = createState(createRuleExecutionInput(), undefined, createEsqlResponse());
9090

91-
await expect(step.execute(state)).rejects.toThrow(
92-
'CreateAlertEventsStep requires rule from previous step'
93-
);
91+
const result = await step.execute(state);
92+
93+
expect(result).toEqual({ type: 'halt', reason: 'state_not_ready' });
9494
});
9595

96-
it('throws when esqlResponse is missing from state', async () => {
96+
it('halts with state_not_ready when esqlResponse is missing from state', async () => {
9797
const { loggerService } = createLoggerService();
9898
const { storageService } = createStorageService();
9999

100100
const step = new CreateAlertEventsStep(loggerService, storageService);
101101
const state = createState(createRuleExecutionInput(), createRuleResponse(), undefined);
102102

103-
await expect(step.execute(state)).rejects.toThrow(
104-
'CreateAlertEventsStep requires esqlResponse from previous step'
105-
);
103+
const result = await step.execute(state);
104+
105+
expect(result).toEqual({ type: 'halt', reason: 'state_not_ready' });
106106
});
107107

108108
it('propagates storage service errors', async () => {

‎x-pack/platform/plugins/shared/alerting_v2/server/lib/rule_executor/steps/create_alert_events_step.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import {
1515
LoggerServiceToken,
1616
type LoggerServiceContract,
1717
} from '../../services/logger_service/logger_service';
18+
import type { StateWithEsqlResponse, StateWithRule } from '../type_guards';
19+
import { hasRuleAndEsqlResponse } from '../type_guards';
1820

1921
@injectable()
2022
export class CreateAlertEventsStep implements RuleExecutionStep {
@@ -25,16 +27,20 @@ export class CreateAlertEventsStep implements RuleExecutionStep {
2527
@inject(StorageServiceInternalToken) private readonly storageService: StorageServiceContract
2628
) {}
2729

30+
private isStepReady(
31+
state: Readonly<RulePipelineState>
32+
): state is StateWithRule & StateWithEsqlResponse {
33+
return hasRuleAndEsqlResponse(state);
34+
}
35+
2836
public async execute(state: Readonly<RulePipelineState>): Promise<RuleStepOutput> {
29-
const { rule, esqlResponse, input } = state;
37+
const { input } = state;
3038

31-
if (!rule) {
32-
throw new Error('CreateAlertEventsStep requires rule from previous step');
39+
if (!this.isStepReady(state)) {
40+
return { type: 'halt', reason: 'state_not_ready' };
3341
}
3442

35-
if (!esqlResponse) {
36-
throw new Error('CreateAlertEventsStep requires esqlResponse from previous step');
37-
}
43+
const { rule, esqlResponse } = state;
3844

3945
const alertEvents = buildAlertEventsFromEsqlResponse({
4046
ruleId: input.ruleId,

‎x-pack/platform/plugins/shared/alerting_v2/server/lib/rule_executor/steps/execute_rule_query_step.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,15 +102,15 @@ describe('ExecuteRuleQueryStep', () => {
102102
await expect(step.execute(state)).rejects.toThrow('Query execution failed');
103103
});
104104

105-
it('throws when rule is missing from state', async () => {
105+
it('halts with state_not_ready when rule is missing from state', async () => {
106106
const { loggerService } = createLoggerService();
107107
const { queryService } = createQueryService();
108108

109109
const step = new ExecuteRuleQueryStep(loggerService, queryService);
110110
const state = createState(createRuleExecutionInput(), undefined);
111111

112-
await expect(step.execute(state)).rejects.toThrow(
113-
'ExecuteRuleQueryStep requires rule from previous step'
114-
);
112+
const result = await step.execute(state);
113+
114+
expect(result).toEqual({ type: 'halt', reason: 'state_not_ready' });
115115
});
116116
});

‎x-pack/platform/plugins/shared/alerting_v2/server/lib/rule_executor/steps/execute_rule_query_step.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import {
1616
QueryService,
1717
type QueryServiceContract,
1818
} from '../../services/query_service/query_service';
19+
import type { StateWithRule } from '../type_guards';
20+
import { hasRule } from '../type_guards';
1921

2022
@injectable()
2123
export class ExecuteRuleQueryStep implements RuleExecutionStep {
@@ -26,13 +28,19 @@ export class ExecuteRuleQueryStep implements RuleExecutionStep {
2628
@inject(QueryService) private readonly queryService: QueryServiceContract
2729
) {}
2830

31+
private isStepReady(state: Readonly<RulePipelineState>): state is StateWithRule {
32+
return hasRule(state);
33+
}
34+
2935
public async execute(state: Readonly<RulePipelineState>): Promise<RuleStepOutput> {
30-
const { rule, input } = state;
36+
const { input } = state;
3137

32-
if (!rule) {
33-
throw new Error('ExecuteRuleQueryStep requires rule from previous step');
38+
if (!this.isStepReady(state)) {
39+
return { type: 'halt', reason: 'state_not_ready' };
3440
}
3541

42+
const { rule } = state;
43+
3644
const queryPayload = getQueryPayload({
3745
query: rule.query,
3846
timeField: rule.timeField,

‎x-pack/platform/plugins/shared/alerting_v2/server/lib/rule_executor/steps/validate_rule_step.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ describe('ValidateRuleStep', () => {
3737
});
3838
});
3939

40-
it('throws when rule is missing from state', async () => {
40+
it('halts with state_not_ready when rule is missing from state', async () => {
4141
const step = new ValidateRuleStep();
4242
const state = createState(undefined);
4343

44-
await expect(step.execute(state)).rejects.toThrow(
45-
'ValidateRuleStep requires rule from previous step'
46-
);
44+
const result = await step.execute(state);
45+
46+
expect(result).toEqual({ type: 'halt', reason: 'state_not_ready' });
4747
});
4848
});

‎x-pack/platform/plugins/shared/alerting_v2/server/lib/rule_executor/steps/validate_rule_step.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,23 @@
77

88
import { injectable } from 'inversify';
99
import type { RuleExecutionStep, RulePipelineState, RuleStepOutput } from '../types';
10+
import type { StateWithRule } from '../type_guards';
11+
import { hasRule } from '../type_guards';
1012

1113
@injectable()
1214
export class ValidateRuleStep implements RuleExecutionStep {
1315
public readonly name = 'validate_rule';
1416

15-
public async execute(state: Readonly<RulePipelineState>): Promise<RuleStepOutput> {
16-
const { rule } = state;
17+
private isStepReady(state: Readonly<RulePipelineState>): state is StateWithRule {
18+
return hasRule(state);
19+
}
1720

18-
if (!rule) {
19-
throw new Error('ValidateRuleStep requires rule from previous step');
21+
public async execute(state: Readonly<RulePipelineState>): Promise<RuleStepOutput> {
22+
if (!this.isStepReady(state)) {
23+
return { type: 'halt', reason: 'state_not_ready' };
2024
}
2125

22-
if (!rule.enabled) {
26+
if (!state.rule.enabled) {
2327
return { type: 'halt', reason: 'rule_disabled' };
2428
}
2529

‎x-pack/platform/plugins/shared/alerting_v2/server/lib/rule_executor/type_guards.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
import type { RulePipelineState } from './types';
9+
10+
export type StateWithRule = RulePipelineState & {
11+
rule: NonNullable<RulePipelineState['rule']>;
12+
};
13+
14+
export type StateWithEsqlResponse = RulePipelineState & {
15+
esqlResponse: NonNullable<RulePipelineState['esqlResponse']>;
16+
};
17+
18+
export function hasRule(state: RulePipelineState): state is StateWithRule {
19+
return state.rule !== undefined;
20+
}
21+
22+
export function hasEsqlResponse(state: RulePipelineState): state is StateWithEsqlResponse {
23+
return state.esqlResponse !== undefined;
24+
}
25+
26+
export function hasRuleAndEsqlResponse(
27+
state: RulePipelineState
28+
): state is StateWithRule & StateWithEsqlResponse {
29+
return hasRule(state) && hasEsqlResponse(state);
30+
}

‎x-pack/platform/plugins/shared/alerting_v2/server/lib/rule_executor/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export interface RulePipelineState {
3030
readonly alertEvents?: Array<{ id: string; doc: AlertEvent }>;
3131
}
3232

33-
export type HaltReason = 'rule_deleted' | 'rule_disabled';
33+
export type HaltReason = 'rule_deleted' | 'rule_disabled' | 'state_not_ready';
3434

3535
export type RuleStepOutput =
3636
| { type: 'continue'; data?: Partial<Omit<RulePipelineState, 'input'>> }

0 commit comments

Comments
 (0)