Skip to content

Commit

Permalink
[kbn-ftr-common-functional-services] extend retry service (elastic#17…
Browse files Browse the repository at this point in the history
…8660)

## Summary

I took the retry wrapper created by
[jpdjere](https://github.com/jpdjere) in elastic#173998 and extended
`retryForSuccess<T>` with required capabilities to provide the same
functionality.

This PR:
1) extends retry service with new function `tryWithRetries<T> => :
Promise<T>` to retry block `options.retryCount` times within
`options.timeout` period and return block result

```ts
  const response = await retry.tryWithRetries<SearchResponse>(
   'search request',
    async () => {
        const response = await supertest
          .post(`/internal/search/es`)
          .set(ELASTIC_HTTP_VERSION_HEADER, '1')
          .send({
            params: {
              body: {
                query: {
                  match_all: {},
                },
              },
            },
          })
          .expect(200);
        return response.body as SearchResponse;
    },
    {
      retryCount: 4,
      retryDelay: 100, // optional
      timeout: 30000, // optional
    }
```

2) removes `utils/retry.ts` wrapper and migrate tests to FTR Retry
service
3) Adds descriptions to Retry service functions explaining the default
use case

How the failures look like:

- when reached timeout before retry count limit
```
 Error: retry.tryWithRetries reached timeout 11000 ms waiting for 'run console request'
 Error: expected '# Click the Variables button, above, to create your own variables.\nGET ${exampleVariable1} // _search\n{\n  "query": {\n    "${exampleVariable2}": {} // match_all\n  }\n}' to sort of equal '5'
     at Assertion.assert (expect.js:100:11)
     at Assertion.eql (expect.js:244:8)
     at _console.ts:47:41
     at processTicksAndRejections (node:internal/process/task_queues:95:5)
     at runAttempt (retry_with_retries.ts:29:15)
     at retryWithRetries (retry_with_retries.ts:97:21)
     at RetryService.tryForTime (retry.ts:38:12)
     at Context.<anonymous> (_console.ts:44:7)
     at Object.apply (wrap_function.js:73:16)
```
- when reached retry count limit before timeout
```
 Error: retry.tryWithRetries reached the limit of attempts waiting for 'run console request': 2 out of 2
 Error: expected '# Click the Variables button, above, to create your own variables.\nGET ${exampleVariable1} // _search\n{\n  "query": {\n    "${exampleVariable2}": {} // match_all\n  }\n}' to sort of equal '5'
     at Assertion.assert (expect.js:100:11)
     at Assertion.eql (expect.js:244:8)
     at _console.ts:47:41
     at processTicksAndRejections (node:internal/process/task_queues:95:5)
     at runAttempt (retry_for_success.ts:29:15)
     at retryForSuccess (retry_for_success.ts:97:21)
     at RetryService.tryWithRetries (retry.ts:115:12)
     at Context.<anonymous> (_console.ts:44:7)
     at Object.apply (wrap_function.js:73:16)
```

(cherry picked from commit 277b3fb)
  • Loading branch information
dmlemeshko committed Mar 19, 2024
1 parent 67bb4b1 commit b74556d
Show file tree
Hide file tree
Showing 10 changed files with 114 additions and 157 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,31 @@ import { FtrService } from '../ftr_provider_context';
import { retryForSuccess } from './retry_for_success';
import { retryForTruthy } from './retry_for_truthy';

interface TryWithRetriesOptions {
retryCount: number;
retryDelay?: number;
timeout?: number;
}

export class RetryService extends FtrService {
private readonly config = this.ctx.getService('config');
private readonly log = this.ctx.getService('log');

/**
* Use to retry block within {timeout} period and return block result.
* @param timeout retrying timeout
* @param block retriable action
* @param onFailureBlock optional action to run before the new retriable action attempt
* @param retryDelay optional delay before the new attempt
* @returns result from retriable action
*/
public async tryForTime<T>(
timeout: number,
block: () => Promise<T>,
onFailureBlock?: () => Promise<T>,
retryDelay?: number
) {
return await retryForSuccess(this.log, {
return await retryForSuccess<T>(this.log, {
timeout,
methodName: 'retry.tryForTime',
block,
Expand All @@ -43,6 +57,13 @@ export class RetryService extends FtrService {
});
}

/**
* Use to wait for block condition to be true
* @param description description for retriable action
* @param timeout retrying timeout
* @param block retriable action
* @param onFailureBlock optional action to run before the new retriable action attempt
*/
public async waitForWithTimeout(
description: string,
timeout: number,
Expand Down Expand Up @@ -71,4 +92,31 @@ export class RetryService extends FtrService {
onFailureBlock,
});
}

/**
* Use to retry block {options.retryCount} times within {options.timeout} period and return block result
* @param description description for retriable action
* @param block retriable action
* @param options options.retryCount for how many attempts to retry
* @param onFailureBlock optional action to run before the new retriable action attempt
* @returns result from retriable action
*/
public async tryWithRetries<T>(
description: string,
block: () => Promise<T>,
options: TryWithRetriesOptions,
onFailureBlock?: () => Promise<T>
): Promise<T> {
const { retryCount, timeout = this.config.get('timeouts.try'), retryDelay = 200 } = options;

return await retryForSuccess<T>(this.log, {
description,
timeout,
methodName: 'retry.tryWithRetries',
block,
onFailureBlock,
retryDelay,
retryCount,
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));

const returnTrue = () => true;

const defaultOnFailure = (methodName: string) => (lastError: Error | undefined) => {
const defaultOnFailure = (methodName: string) => (lastError: Error | undefined, reason: string) => {
throw new Error(
`${methodName} timeout${lastError ? `: ${lastError.stack || lastError.message}` : ''}`
`${methodName} ${reason}\n${lastError ? `${lastError.stack || lastError.message}` : ''}`
);
};

Expand Down Expand Up @@ -44,32 +44,51 @@ interface Options<T> {
onFailureBlock?: () => Promise<T>;
onFailure?: ReturnType<typeof defaultOnFailure>;
accept?: (v: T) => boolean;
description?: string;
retryDelay?: number;
retryCount?: number;
}

export async function retryForSuccess<T>(log: ToolingLog, options: Options<T>) {
const {
description,
timeout,
methodName,
block,
onFailureBlock,
onFailure = defaultOnFailure(methodName),
accept = returnTrue,
retryDelay = 502,
retryCount,
} = options;
const { onFailure = defaultOnFailure(methodName) } = options;

const start = Date.now();
const criticalWebDriverErrors = ['NoSuchSessionError', 'NoSuchWindowError'];
let lastError;
let attemptCounter = 0;
const addText = (str: string | undefined) => (str ? ` waiting for '${str}'` : '');

while (true) {
// Aborting if no retry attempts are left (opt-in)
if (retryCount && ++attemptCounter > retryCount) {
onFailure(
lastError,
// optionally extend error message with description
`reached the limit of attempts${addText(description)}: ${
attemptCounter - 1
} out of ${retryCount}`
);
}
// Aborting if timeout is reached
if (Date.now() - start > timeout) {
await onFailure(lastError);
throw new Error('expected onFailure() option to throw an error');
} else if (lastError && criticalWebDriverErrors.includes(lastError.name)) {
// Aborting retry since WebDriver session is invalid or browser window is closed
onFailure(lastError, `reached timeout ${timeout} ms${addText(description)}`);
}
// Aborting if WebDriver session is invalid or browser window is closed
if (lastError && criticalWebDriverErrors.includes(lastError.name)) {
throw new Error('WebDriver session is invalid, retry was aborted');
} else if (lastError && onFailureBlock) {
}
// Run opt-in onFailureBlock before the next attempt
if (lastError && onFailureBlock) {
const before = await runAttempt(onFailureBlock);
if ('error' in before) {
log.debug(`--- onRetryBlock error: ${before.error.message}`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ export default ({ getService }: FtrProviderContext): void => {
es,
supertest,
'99.0.0',
retry,
log
retry
);

// As opposed to "registry"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ export default ({ getService }: FtrProviderContext): void => {
const fleetPackageInstallationResponse = await installPrebuiltRulesPackageViaFleetAPI(
es,
supertest,
retry,
log
retry
);

expect(fleetPackageInstallationResponse.items.length).toBe(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ export default ({ getService }: FtrProviderContext): void => {
supertest,
overrideExistingPackage: true,
retryService: retry,
log,
});

// Verify that status is updated after package installation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ export default ({ getService }: FtrProviderContext): void => {
es,
supertest,
previousVersion,
retry,
log
retry
);

expect(installPreviousPackageResponse._meta.install_source).toBe('registry');
Expand Down Expand Up @@ -161,8 +160,7 @@ export default ({ getService }: FtrProviderContext): void => {
es,
supertest,
currentVersion,
retry,
log
retry
);
expect(installLatestPackageResponse.items.length).toBeGreaterThanOrEqual(0);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,4 @@ export * from './wait_for_index_to_populate';
export * from './get_stats';
export * from './get_detection_metrics_from_body';
export * from './get_stats_url';
export * from './retry';
export * from './combine_to_ndjson';

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ import type SuperTest from 'supertest';
import { InstallPackageResponse } from '@kbn/fleet-plugin/common/types';
import { epmRouteService } from '@kbn/fleet-plugin/common';
import { RetryService } from '@kbn/ftr-common-functional-services';
import type { ToolingLog } from '@kbn/tooling-log';
import expect from 'expect';
import { retry } from '../../retry';
import { refreshSavedObjectIndices } from '../../refresh_index';

const MAX_RETRIES = 2;
Expand All @@ -29,11 +27,11 @@ const ATTEMPT_TIMEOUT = 120000;
export const installPrebuiltRulesPackageViaFleetAPI = async (
es: Client,
supertest: SuperTest.SuperTest<SuperTest.Test>,
retryService: RetryService,
log: ToolingLog
retryService: RetryService
): Promise<InstallPackageResponse> => {
const fleetResponse = await retry<InstallPackageResponse>({
test: async () => {
const fleetResponse = await retryService.tryWithRetries<InstallPackageResponse>(
installPrebuiltRulesPackageViaFleetAPI.name,
async () => {
const testResponse = await supertest
.post(`/api/fleet/epm/packages/security_detection_engine`)
.set('kbn-xsrf', 'xxxx')
Expand All @@ -46,12 +44,11 @@ export const installPrebuiltRulesPackageViaFleetAPI = async (

return testResponse.body;
},
utilityName: installPrebuiltRulesPackageViaFleetAPI.name,
retryService,
retries: MAX_RETRIES,
timeout: ATTEMPT_TIMEOUT,
log,
});
{
retryCount: MAX_RETRIES,
timeout: ATTEMPT_TIMEOUT,
}
);

await refreshSavedObjectIndices(es);

Expand All @@ -71,11 +68,11 @@ export const installPrebuiltRulesPackageByVersion = async (
es: Client,
supertest: SuperTest.SuperTest<SuperTest.Test>,
version: string,
retryService: RetryService,
log: ToolingLog
retryService: RetryService
): Promise<InstallPackageResponse> => {
const fleetResponse = await retry<InstallPackageResponse>({
test: async () => {
const fleetResponse = await retryService.tryWithRetries<InstallPackageResponse>(
installPrebuiltRulesPackageByVersion.name,
async () => {
const testResponse = await supertest
.post(epmRouteService.getInstallPath('security_detection_engine', version))
.set('kbn-xsrf', 'xxxx')
Expand All @@ -88,12 +85,11 @@ export const installPrebuiltRulesPackageByVersion = async (

return testResponse.body;
},
utilityName: installPrebuiltRulesPackageByVersion.name,
retryService,
retries: MAX_RETRIES,
timeout: ATTEMPT_TIMEOUT,
log,
});
{
retryCount: MAX_RETRIES,
timeout: ATTEMPT_TIMEOUT,
}
);

await refreshSavedObjectIndices(es);

Expand Down
Loading

0 comments on commit b74556d

Please sign in to comment.