Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API errors expects new error format #93

Merged
merged 6 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/sandboxHubs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const SANDBOX_API_PATH_V2 = 'sandbox-hubs/v2';
export async function createSandbox(
accountId: number,
name: string,
type: string
type: 1 | 2
): Promise<SandboxResponse> {
return http.post(accountId, {
data: { name, type, generatePersonalAccessKey: true }, // For CLI, generatePersonalAccessKey will always be true since we'll be saving the entry to the config
Expand Down
67 changes: 57 additions & 10 deletions errors/__tests__/apiErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
getAxiosErrorWithContext,
throwApiError,
throwApiUploadError,
isSpecifiedError,
} from '../apiErrors';
import { BaseError } from '../../types/Error';

Expand Down Expand Up @@ -42,19 +43,68 @@ export const newAxiosError = (overrides = {}): AxiosError => {
};

describe('errors/apiErrors', () => {
describe('isSpecifiedError()', () => {
it('returns true for a matching specified error', () => {
const error1 = newAxiosError({
response: {
status: 403,
data: { category: 'BANNED', subCategory: 'USER_ACCESS_NOT_ALLOWED' },
},
});
expect(
isSpecifiedError(error1, {
statusCode: 403,
category: 'BANNED',
subCategory: 'USER_ACCESS_NOT_ALLOWED',
})
).toBe(true);
});

it('returns false for non matching specified errors', () => {
const error1 = newAxiosError({
response: {
status: 403,
data: { category: 'BANNED', subCategory: 'USER_ACCESS_NOT_ALLOWED' },
},
});
const error2 = newAxiosError({ isAxiosError: false });
expect(
isSpecifiedError(error1, {
statusCode: 400,
category: 'GATED',
})
).toBe(false);
expect(isMissingScopeError(error2)).toBe(false);
});

it('handles AxiosError returned in cause property', () => {
const axiosError = newAxiosError({
response: {
status: 403,
data: { category: 'BANNED', subCategory: 'USER_ACCESS_NOT_ALLOWED' },
},
});
const error1 = newError({ cause: axiosError });
expect(
isSpecifiedError(error1, {
statusCode: 403,
category: 'BANNED',
subCategory: 'USER_ACCESS_NOT_ALLOWED',
})
).toBe(true);
});
});
describe('isMissingScopeError()', () => {
it('returns true for missing scope errors', () => {
const error1 = newAxiosError({
status: 403,
response: { data: { category: 'MISSING_SCOPES' } },
response: { status: 403, data: { category: 'MISSING_SCOPES' } },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw the AxiosError type definition does include a status property, but I wasn't able to log any statuses directly from error when testing with actual API calls... I had to go one level deeper into the response (AxiosResponse type) to pull status, hence the changes to the tests here

});
expect(isMissingScopeError(error1)).toBe(true);
});

it('returns false for non missing scope errors', () => {
const error1 = newAxiosError({
status: 400,
response: { data: { category: 'MISSING_SCOPES' } },
response: { status: 400, data: { category: 'MISSING_SCOPES' } },
});
const error2 = newAxiosError({ isAxiosError: false });
expect(isMissingScopeError(error1)).toBe(false);
Expand All @@ -65,16 +115,14 @@ describe('errors/apiErrors', () => {
describe('isGatingError()', () => {
it('returns true for gating errors', () => {
const error1 = newAxiosError({
status: 403,
response: { data: { category: 'GATED' } },
response: { status: 403, data: { category: 'GATED' } },
});
expect(isGatingError(error1)).toBe(true);
});

it('returns false for non gating errors', () => {
const error1 = newAxiosError({
status: 400,
response: { data: { category: 'GATED' } },
response: { status: 400, data: { category: 'GATED' } },
});
const error2 = newAxiosError({ isAxiosError: false });
expect(isGatingError(error1)).toBe(false);
Expand All @@ -98,8 +146,7 @@ describe('errors/apiErrors', () => {

it('returns false for non api upload validation errors', () => {
const error1 = newAxiosError({
status: 400,
response: { data: null },
response: { status: 400, data: null },
});
const error2 = newAxiosError({ isAxiosError: false });
expect(isApiUploadValidationError(error1)).toBe(false);
Expand Down
38 changes: 22 additions & 16 deletions errors/apiErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,30 @@ import { HttpMethod } from '../types/Api';

const i18nKey = 'errors.apiErrors';

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function isMissingScopeError(err: AxiosError<any>): boolean {
return (
err.isAxiosError &&
err.status === 403 &&
!!err.response &&
err.response.data.category === 'MISSING_SCOPES'
);
export function isSpecifiedError(
err: Error | AxiosError,
{
statusCode,
category,
subCategory,
}: { statusCode?: number; category?: string; subCategory?: string }
): boolean {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const error = (err && (err.cause as AxiosError<any>)) || err;
const statusCodeErr = !statusCode || error.response?.status === statusCode;
const categoryErr = !category || error.response?.data?.category === category;
const subCategoryErr =
!subCategory || error.response?.data?.subCategory === subCategory;

return error.isAxiosError && statusCodeErr && categoryErr && subCategoryErr;
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function isGatingError(err: AxiosError<any>): boolean {
return (
err.isAxiosError &&
err.status === 403 &&
!!err.response &&
err.response.data.category === 'GATED'
);
export function isMissingScopeError(err: Error | AxiosError): boolean {
return isSpecifiedError(err, { statusCode: 403, category: 'MISSING_SCOPES' });
}

export function isGatingError(err: Error | AxiosError): boolean {
return isSpecifiedError(err, { statusCode: 403, category: 'GATED' });
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down
4 changes: 2 additions & 2 deletions errors/standardErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function genericThrowErrorWithMessage(
ErrorType: ErrorConstructor,
identifier: LangKey,
interpolation?: { [key: string]: string | number },
cause?: BaseError
cause?: BaseError | AxiosError
): never {
const message = i18n(identifier, interpolation);
if (cause) {
Expand All @@ -32,7 +32,7 @@ function genericThrowErrorWithMessage(
export function throwErrorWithMessage(
identifier: LangKey,
interpolation?: { [key: string]: string | number },
cause?: BaseError
cause?: BaseError | AxiosError
): never {
genericThrowErrorWithMessage(Error, identifier, interpolation, cause);
}
Expand Down
10 changes: 0 additions & 10 deletions lang/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,6 @@
"invalidPersonalAccessKey": "Error while retrieving new access token: {{ errorMessage }}"
}
},
"sandboxes": {
"errors": {
"createSandbox": "There was an error creating your sandbox.",
"deleteSandbox": "There was an error deleting your sandbox.",
"getSandboxUsageLimits": "There was an error fetching sandbox usage limits.",
"initiateSync": "There was an error initiating the sandbox sync.",
"fetchTaskStatus": "There was an error fetching the task status while syncing sandboxes.",
"fetchTypes": "There was an error fetching sandbox types."
}
},
"cms": {
"modules": {
"createModule": {
Expand Down
108 changes: 108 additions & 0 deletions lib/__tests__/sandboxes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding these! It's a huge help 🙏

createSandbox as __createSandbox,
getSandboxUsageLimits as __getSandboxUsageLimits,
} from '../../api/sandboxHubs';
import { fetchTypes as __fetchTypes } from '../../api/sandboxSync';
import { Sandbox, Usage } from '../../types/Sandbox';
import {
createSandbox as createSandboxAction,
deleteSandbox as deleteSandboxAction,
getSandboxUsageLimits as getSandboxUsageLimitsAction,
fetchTypes as fetchTypesAction,
} from '../sandboxes';

jest.mock('../../api/sandboxHubs');
jest.mock('../../api/sandboxSync');

const createSandbox = __createSandbox as jest.MockedFunction<
typeof __createSandbox
>;
const getSandboxUsageLimits = __getSandboxUsageLimits as jest.MockedFunction<
typeof __getSandboxUsageLimits
>;
const fetchTypes = __fetchTypes as jest.MockedFunction<typeof __fetchTypes>;

const sandboxName = 'Mock Standard Sandbox';
const sandboxHubId = 987654;
const accountId = 123456;

const MOCK_SANDBOX_ACCOUNT: Sandbox = {
sandboxHubId: sandboxHubId,
parentHubId: accountId,
createdAt: '2023-01-27T22:24:27.279Z',
updatedAt: '2023-02-09T19:36:25.123Z',
archivedAt: null,
type: 'developer',
archived: false,
name: sandboxName,
domain: 'mockStandardSandbox.com',
createdByUser: {
userId: 111,
firstName: 'Test',
lastName: 'User',
},
};

const MOCK_USAGE_DATA: Usage = {
STANDARD: {
used: 0,
available: 1,
limit: 1,
},
DEVELOPER: {
used: 0,
available: 1,
limit: 1,
},
};

const MOCK_TYPES = [
{
name: 'object-schemas',
dependsOn: [],
pushToProductionEnabled: true,
isBeta: false,
diffEnabled: true,
groupType: 'object-schemas',
syncMandatory: true,
},
];

describe('lib/sandboxes', () => {
it('createSandbox()', async () => {
const personalAccessKey = 'pak-test-123';
createSandbox.mockResolvedValue({
sandbox: MOCK_SANDBOX_ACCOUNT,
personalAccessKey,
});

const response = await createSandboxAction(accountId, sandboxName, 1);
expect(response.personalAccessKey).toEqual(personalAccessKey);
expect(response.name).toEqual(sandboxName);
expect(response.sandbox).toBeTruthy();
});

it('deleteSandbox()', async () => {
const response = await deleteSandboxAction(accountId, sandboxHubId);
expect(response.parentAccountId).toEqual(accountId);
expect(response.sandboxAccountId).toEqual(sandboxHubId);
});

it('getSandboxUsageLimits()', async () => {
getSandboxUsageLimits.mockResolvedValue({
usage: MOCK_USAGE_DATA,
});

const response = await getSandboxUsageLimitsAction(accountId);
expect(response).toMatchObject(MOCK_USAGE_DATA);
});

it('fetchTypes()', async () => {
fetchTypes.mockResolvedValue({
results: MOCK_TYPES,
});

const response = await fetchTypesAction(accountId, sandboxHubId);
expect(response).toMatchObject(MOCK_TYPES);
});
});
Loading
Loading