Skip to content
This repository has been archived by the owner on May 27, 2021. It is now read-only.

Commit

Permalink
Merge #972
Browse files Browse the repository at this point in the history
972: Refactor requests to allow for non Normandy requests r=jaredkerim a=rehandalal

Blocks #943 

r?

Co-authored-by: Rehan Dalal <rehandalal@gmail.com>
  • Loading branch information
bors[bot] and rehandalal committed Jul 30, 2019
2 parents 5b2cad3 + 6b74e89 commit d5144a1
Show file tree
Hide file tree
Showing 7 changed files with 139 additions and 101 deletions.
48 changes: 48 additions & 0 deletions src/state/network/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,57 @@ import {
import { getAccessToken, isInsecureAuth } from 'console/state/auth/selectors';
import { getRequest } from 'console/state/network/selectors';
import APIClient from 'console/utils/api';
import { request } from 'console/utils/request';

const SECONDS = 1000;

export function makeRequest(requestId, url, options = {}) {
return async (dispatch, getState) => {
const state = getState();
const prevRequest = getRequest(state, requestId);

// A "stealth API request" is one that doesn't update the state.
// This is useful when you generally "don't care" about the *state* of the request.
const stealth = options.stealth || false;
if (stealth) {
// Doing it this way because JavaScript doesn't have a `obj.pop(key, default)`
delete options.stealth;
}

if (prevRequest.inProgress) {
return true;
}

if (!stealth) {
dispatch({
type: REQUEST_SEND,
requestId,
});
}

let data;

try {
data = await request(url, options);
} catch (error) {
dispatch({
type: REQUEST_FAILURE,
requestId,
error,
});

throw error;
}

dispatch({
type: REQUEST_SUCCESS,
requestId,
});

return data;
};
}

export function makeApiRequest(requestId, root, endpoint = '', options = {}) {
return async (dispatch, getState) => {
const state = getState();
Expand Down
21 changes: 1 addition & 20 deletions src/tests/utils/api.test.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,8 @@
import APIClient, { APIError } from 'console/utils/api';
import APIClient from 'console/utils/api';

describe('APIClient util', () => {
it('should work', () => {
const wrapper = () => new APIClient();
expect(wrapper).not.toThrow();
});
});

describe('APIError', () => {
it('should identify as an APIError', () => {
const errMessage = 'Danger, Will Robinson! Danger!';
const errData = { danger: true };
let didThrow = false;

try {
throw new APIError(errMessage, errData);
} catch (e) {
didThrow = true;
expect(e.name).toBe('APIError');
expect(e.message).toBe(errMessage);
expect(e.data).toBe(errData);
}

expect(didThrow).toBe(true);
});
});
18 changes: 9 additions & 9 deletions src/tests/utils/handleError.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { APIError } from 'console/utils/api';
import { ValidationError } from 'console/utils/forms';
import handleError, { ERR_MESSAGES } from 'console/utils/handleError';
import { checkAPIFailure, checkValidationFailure } from 'console/utils/handleError';
import { RequestError } from 'console/utils/request';

describe('handleError util', () => {
it('should work', () => {
Expand Down Expand Up @@ -32,9 +32,9 @@ describe('handleError util', () => {
expect(reason).toBe(ERR_MESSAGES.FORM_VALIDATION);
});

describe('API Errors', () => {
it('should detect APIErrors', () => {
const err = new APIError('Something from the server.', {
describe('Request Errors', () => {
it('should detect RequestErrors', () => {
const err = new RequestError('Something from the server.', {
status: 400,
});

Expand All @@ -48,7 +48,7 @@ describe('handleError util', () => {
});

it('should handle 400 errors', () => {
const err = new APIError('Something from the server.', {
const err = new RequestError('Something from the server.', {
status: 400,
});

Expand All @@ -60,7 +60,7 @@ describe('handleError util', () => {

describe('should handle 403 errors', () => {
it('should handle a "not logged in" 403 error', () => {
const err = new APIError('Authentication credentials were not provided', {
const err = new RequestError('Authentication credentials were not provided', {
status: 403,
});

Expand All @@ -71,7 +71,7 @@ describe('handleError util', () => {
});

it('should handle a "no permission" 403 error', () => {
const err = new APIError('User does not have permission to perform that action.', {
const err = new RequestError('User does not have permission to perform that action.', {
status: 403,
});

Expand All @@ -83,7 +83,7 @@ describe('handleError util', () => {
});

it('should handle 500 errors', () => {
const err = new APIError('Something from the server.', {
const err = new RequestError('Something from the server.', {
status: 500,
});

Expand All @@ -94,7 +94,7 @@ describe('handleError util', () => {
});

it('should fall back to server messages if the response status is unrecognized', () => {
const err = new APIError('Something from the server.', {
const err = new RequestError('Something from the server.', {
status: 123,
});

Expand Down
75 changes: 6 additions & 69 deletions src/utils/api.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
import { INSECURE_AUTH_ALLOWED } from 'console/settings';

export function APIError(message, data = {}) {
this.data = data;
this.message = message;
this.stack = Error().stack;
}
APIError.prototype = Object.create(Error.prototype);
APIError.prototype.name = 'APIError';
import { request } from 'console/utils/request';

export default class APIClient {
constructor(root, accessToken, insecure = false) {
Expand All @@ -15,73 +8,17 @@ export default class APIClient {
this.insecure = insecure;
}

async fetch(url, options) {
let queryString = '';

const headers = new Headers();
headers.append('Accept', 'application/json');
if (!(options.body && options.body instanceof FormData)) {
headers.append('Content-Type', 'application/json');
}
fetch(url, options = {}) {
const extraHeaders = {};

if (this.accessToken) {
if (INSECURE_AUTH_ALLOWED && this.insecure) {
headers.append('Authorization', `Insecure ${this.accessToken}`);
} else {
headers.append('Authorization', `Bearer ${this.accessToken}`);
}
}

const settings = {
headers,
credentials: 'same-origin',
method: 'GET',
...options,
};

// Convert `data` to `body` or querystring if necessary.
if ('data' in settings) {
if ('body' in settings) {
throw new Error('Only pass one of `settings.data` and `settings.body`.');
}

if (['GET', 'HEAD'].includes(settings.method.toUpperCase())) {
const queryStringData = Object.keys(settings.data).map(
key => `${key}=${encodeURIComponent(settings.data[key])}`,
);
queryString = `?${queryStringData.join('&')}`;
extraHeaders.Authorization = `Insecure ${this.accessToken}`;
} else {
settings.body = JSON.stringify(settings.data);
extraHeaders.Authorization = `Bearer ${this.accessToken}`;
}

delete settings.data;
}

const response = await fetch(`${this.root}${url}${queryString}`, settings);

// Throw if we get a non-200 response.
if (!response.ok) {
let message;
let data = {};
let err;

try {
data = await response.json();
message = data.detail || response.statusText;
} catch (error) {
message = error.message;
err = error;
}

data = { ...data, status: response.status };

throw new APIError(message, data, err);
}

if (response.status !== 204) {
return response.json();
}

return null;
return request(`${this.root}${url}`, options, extraHeaders);
}
}
4 changes: 2 additions & 2 deletions src/utils/handleError.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { notification } from 'antd';

import { APIError } from 'console/utils/api';
import { RequestError } from 'console/utils/request';
import { ValidationError } from 'console/utils/forms';

export const ERR_MESSAGES = {
Expand All @@ -17,7 +17,7 @@ const checkFetchFailure = ({ message = '' }) =>

const checkLoginFailure = ({ message = '' }) =>
message.indexOf('credentials were not provided') > -1;
export const checkAPIFailure = error => error instanceof APIError;
export const checkAPIFailure = error => error instanceof RequestError;
export const checkValidationFailure = error => error instanceof ValidationError;

const msgDisplayTime = 8; // seconds
Expand Down
72 changes: 72 additions & 0 deletions src/utils/request.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
export async function request(url, options = {}, extraHeaders = {}) {
let queryString = '';

const headers = new Headers();
headers.append('Accept', 'application/json');
if (!(options.body && options.body instanceof FormData)) {
headers.append('Content-Type', 'application/json');
}

for (let headerName in extraHeaders) {
headers.append(headerName, extraHeaders[headerName]);
}

const settings = {
headers,
credentials: 'same-origin',
method: 'GET',
...options,
};

// Convert `data` to `body` or querystring if necessary.
if ('data' in settings) {
if ('body' in settings) {
throw new Error('Only pass one of `settings.data` and `settings.body`.');
}

if (['GET', 'HEAD'].includes(settings.method.toUpperCase())) {
const queryStringData = Object.keys(settings.data).map(
key => `${key}=${encodeURIComponent(settings.data[key])}`,
);
queryString = `?${queryStringData.join('&')}`;
} else {
settings.body = JSON.stringify(settings.data);
}

delete settings.data;
}

const response = await fetch(`${url}${queryString}`, settings);

if (!response.ok) {
let message;
let data = {};
let err;

try {
data = await response.json();
message = data.detail || response.statusText;
} catch (error) {
message = error.message;
err = error;
}

data = { ...data, status: response.status };

throw new RequestError(message, data, err);
}

if (response.status !== 204) {
return response.json();
}

return null;
}

export function RequestError(message, data = {}) {
this.data = data;
this.message = message;
this.stack = Error().stack;
}
RequestError.prototype = Object.create(Error.prototype);
RequestError.prototype.name = 'RequestError';
2 changes: 1 addition & 1 deletion src/workflows/recipes/pages/CreateRecipePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class CreateRecipePage extends React.PureComponent {
};

onFormFailure(err) {
handleError('Recipe cannot be created.', err);
handleError(`Recipe cannot be created.`, err);
}

onFormSuccess(recipeId) {
Expand Down

0 comments on commit d5144a1

Please sign in to comment.