Skip to content
This repository has been archived by the owner on Aug 7, 2024. It is now read-only.

Commit

Permalink
Merge pull request #80 from JupiterOne/INT-4045-disabled-step-reason
Browse files Browse the repository at this point in the history
Int 4045 disabled step reason
  • Loading branch information
VDubber authored Jul 1, 2022
2 parents 41616dc + 51ab0f3 commit bab5430
Show file tree
Hide file tree
Showing 6 changed files with 1,215 additions and 1,726 deletions.
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@
"@slack/web-api": "^6.6.0"
},
"devDependencies": {
"@jupiterone/integration-sdk-core": "^7.4.0",
"@jupiterone/integration-sdk-dev-tools": "^7.4.0",
"@jupiterone/integration-sdk-testing": "^7.4.0",
"@jupiterone/integration-sdk-core": "^8.18.0",
"@jupiterone/integration-sdk-dev-tools": "^8.18.0",
"@jupiterone/integration-sdk-testing": "^8.18.0",
"@types/uuid": "^8.3.1",
"type-fest": "^1.3.0",
"uuid": "^8.3.2"
},
"peerDependencies": {
"@jupiterone/integration-sdk-core": "^7.4.0"
"@jupiterone/integration-sdk-core": "^8.18.0"
}
}
17 changes: 14 additions & 3 deletions src/__tests__/getStepStartStates.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import getStepStartStates from '../getStepStartStates';
import { createMockStepExecutionContext } from '../../test/context';
import { USERS_READ_SCOPE, CHANNELS_READ_SCOPE } from '../provider';
import { StepStartStates } from '@jupiterone/integration-sdk-core';
import { CHANNELS_READ_SCOPE, USERS_READ_SCOPE } from '../provider';
import {
DisabledStepReason,
StepStartStates,
} from '@jupiterone/integration-sdk-core';

function getDefaultStepStartStates(overrideStates?: {
[key: string]: { [prop: string]: unknown };
Expand All @@ -12,12 +15,15 @@ function getDefaultStepStartStates(overrideStates?: {
},
'fetch-channels': {
disabled: true,
disabledReason: DisabledStepReason.PERMISSION,
},
'fetch-channel-members': {
disabled: true,
disabledReason: DisabledStepReason.PERMISSION,
},
'fetch-users': {
disabled: true,
disabledReason: DisabledStepReason.PERMISSION,
},
...overrideStates,
};
Expand Down Expand Up @@ -63,13 +69,14 @@ test('should include "fetch-users" step if correct correct scopes provided', ()
const expectedStepStartStates = getDefaultStepStartStates({
'fetch-users': {
disabled: false,
disabledReason: DisabledStepReason.PERMISSION,
},
});

expect(getStepStartStates(context)).toEqual(expectedStepStartStates);
});

test('should include "fetch-channel-members" step if correct correct scopes provided', () => {
test('should include "fetch-channel-members" step if correct scopes provided', () => {
const context = createMockStepExecutionContext({
partialInstanceConfig: {
scopes: `${USERS_READ_SCOPE},${CHANNELS_READ_SCOPE}`,
Expand All @@ -79,12 +86,15 @@ test('should include "fetch-channel-members" step if correct correct scopes prov
const expectedStepStartStates = getDefaultStepStartStates({
'fetch-channel-members': {
disabled: false,
disabledReason: DisabledStepReason.PERMISSION,
},
'fetch-channels': {
disabled: false,
disabledReason: DisabledStepReason.PERMISSION,
},
'fetch-users': {
disabled: false,
disabledReason: DisabledStepReason.PERMISSION,
},
});

Expand All @@ -101,6 +111,7 @@ test('should disable "fetch-channel-members" step if "fetch-users" scopes are no
const expectedStepStartStates = getDefaultStepStartStates({
'fetch-channels': {
disabled: false,
disabledReason: DisabledStepReason.PERMISSION,
},
});

Expand Down
8 changes: 7 additions & 1 deletion src/getStepStartStates.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
DisabledStepReason,
IntegrationExecutionContext,
StepStartStates,
IntegrationValidationError,
Expand Down Expand Up @@ -46,13 +47,18 @@ export default function getStepStartStates(

return {
[teamStep.id]: { disabled: false },
[fetchUsersStep.id]: { disabled: !scopes.has(USERS_READ_SCOPE) },
[fetchUsersStep.id]: {
disabled: !scopes.has(USERS_READ_SCOPE),
disabledReason: DisabledStepReason.PERMISSION,
},
[fetchChannelMembersStep.id]: {
disabled:
!scopes.has(CHANNELS_READ_SCOPE) || !scopes.has(USERS_READ_SCOPE),
disabledReason: DisabledStepReason.PERMISSION,
},
[fetchChannels.id]: {
disabled: !scopes.has(CHANNELS_READ_SCOPE),
disabledReason: DisabledStepReason.PERMISSION,
},
};
}
22 changes: 14 additions & 8 deletions src/provider/SlackProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export interface ListChannelsResult extends WebAPICallResult {
channels: SlackChannel[];
}

const retryOptions = {
const defaultRetryOptions = {
retries: 5,
factor: 1.75, // 1, 1.75, 3.06, 5.35, 9.38
minTimeout: 1000,
Expand All @@ -36,8 +36,13 @@ const retryOptions = {

export class SlackWebClient extends WebClient {
private integrationLogger: IntegrationLogger;
private retryOptions;

constructor(logger: IntegrationLogger, token: string) {
constructor(
logger: IntegrationLogger,
token: string,
retryOptions = defaultRetryOptions,
) {
const webClientOptions: WebClientOptions = {
/**
* The slack client takes optional `logger` and `logLevel` arguments in order to
Expand Down Expand Up @@ -73,16 +78,17 @@ export class SlackWebClient extends WebClient {

super(token, webClientOptions);
this.integrationLogger = logger;
this.retryOptions = { ...defaultRetryOptions, ...retryOptions };
}

async retryWebApiPlatformError<T>(callback: () => Promise<T>) {
return retry(callback, {
delay: retryOptions.minTimeout,
maxAttempts: retryOptions.retries,
jitter: retryOptions.randomize,
factor: retryOptions.factor,
minDelay: retryOptions.minTimeout,
maxDelay: retryOptions.maxTimeout,
delay: this.retryOptions.minTimeout,
maxAttempts: this.retryOptions.retries,
jitter: this.retryOptions.randomize,
factor: this.retryOptions.factor,
minDelay: this.retryOptions.minTimeout,
maxDelay: this.retryOptions.maxTimeout,
handleError: (err, context) => {
if (err.code === ErrorCode.PlatformError) {
this.integrationLogger.warn(
Expand Down
41 changes: 13 additions & 28 deletions src/provider/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@ import {
platformErrorFromResult,
requestErrorWithOriginal,
} from '@slack/web-api/dist/errors';

function flushPromises() {
return new Promise((resolve) => setImmediate(resolve));
}
import { SlackWebClient } from '../SlackProvider';

test('should log error message when rate limited', () => {
const context = createMockStepExecutionContext<SlackIntegrationConfig>();
Expand All @@ -35,41 +32,29 @@ test('should log error message when rate limited', () => {

test('should retry slack_webapi_platform_error 5 times', async () => {
const context = createMockStepExecutionContext<SlackIntegrationConfig>();
const slackClient = createSlackClient(context);

const slackClient = new SlackWebClient(
context.logger,
context.instance.config.accessToken,
{
retries: 5,
factor: 1,
minTimeout: 1,
maxTimeout: 0,
randomize: false,
},
);
const err = platformErrorFromResult({ ok: false, error: '' });

// eslint-disable-next-line @typescript-eslint/require-await
const callback = jest.fn(async () => {
throw err;
});

jest.useFakeTimers();
slackClient.retryWebApiPlatformError(callback).catch((e) => {
await slackClient.retryWebApiPlatformError(callback).catch((e) => {
expect(e).toBe(err);
});

for (const _ of Array(5)) {
/**
* The `retryWebApiPlatformError` function is designed to retry 5 times
* before failing.
*
* Each iteration of `retryWebApiPlatformError()` requires four promises
* to be resolved, and one timer to resolve:
* - await slackClient.retryWebApiPlatformError()
* - await @lifeomic/attempt.retry(attemptFunc)
* - await attemptFunc()
* - await sleep() [also requires a timer to resolve]
*
* For each retry, we must resolve all four promises _and_ run the sleep()
* timer to completion. Only then will the failed callback trigger another
* retry.
*/
jest.runAllTimers();
await flushPromises();
}
expect(callback).toHaveBeenCalledTimes(5);
jest.useRealTimers();
});

test('should not retry slack_webapi_request_error', async () => {
Expand Down
Loading

0 comments on commit bab5430

Please sign in to comment.