Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
77 commits
Select commit Hold shift + click to select a range
4a0a951
feat: add centralized http module containing generic request method a…
Feb 24, 2020
93b41d1
http: implement helper methods
Feb 24, 2020
e44f489
refactor: remove ties to other parts of the app, always throw on error
Feb 27, 2020
3503b8a
update request method signature
Feb 27, 2020
f707d5c
refactor: seperate generic http logic and client specific request mec…
Feb 27, 2020
8d0f01c
fix: namings and method bindings
Mar 1, 2020
fff8e4f
refactor: use ts
Mar 1, 2020
b159d6c
refactor: move code into own folder
Mar 1, 2020
dc05048
refactor: use typescript, move code to own module
Mar 1, 2020
30aa878
start adding tests
Mar 1, 2020
21e87bc
Merge remote-tracking branch 'origin/feature/centralize_api_logic' in…
Mar 1, 2020
39fa7eb
use ky-universal in tests
Mar 3, 2020
94c3584
add timeout option to request config
Mar 3, 2020
2565295
stop working on this for now
Mar 3, 2020
6b8b5b9
rewrite
Mar 3, 2020
32dd5aa
refactor request method signature: split ky and callback options
Mar 5, 2020
de899d9
fix: options merging
Mar 5, 2020
99c512d
stop working for today
Mar 5, 2020
3fd0768
add todo item
Mar 7, 2020
60140e1
fix(OverviewMapState.unit.test.js): HTPPError not exported from ky
Mar 7, 2020
b58b842
add proper types for fetch
Mar 7, 2020
5a714e9
improve(UserState): add token selector
Mar 8, 2020
6bf0371
further implementation
Mar 8, 2020
ceae825
further implement feature
Mar 9, 2020
4395a6a
update stackoverflow link
Mar 9, 2020
0c9039b
chore(jest setup): require dotenv
Mar 19, 2020
3f18d0b
refactor: rename main file to apiService
Mar 19, 2020
dbe66a5
test: correctly test post req
Mar 19, 2020
51203fc
refactor(apiService): clean up
Mar 21, 2020
53c3c75
continue working on error handling
Mar 21, 2020
42c3797
add todo-test
Mar 22, 2020
e4a390f
refactor: rename BodyType to ResponseBodyType
Mar 22, 2020
6946c90
improve: set appropriate request headers for post requests fetching json
Mar 22, 2020
ca35704
refactor(api.unit.test): restructure suites
Mar 22, 2020
f5198af
refactor signature of generic request handler to take an object as se…
Mar 22, 2020
faa9dea
further shape things
s-pic Mar 22, 2020
0ba8a94
improve(auth header test): actually test headers
s-pic Mar 23, 2020
2f996dd
refactor(type): prettify
s-pic Mar 23, 2020
a5c38d4
fix parsing of error response body, make all current tests pass
s-pic Mar 23, 2020
47e380e
chore: add missing dotenv devDependency
s-pic Mar 24, 2020
0ddb665
improve(apiService): in error response parsing do not rely on respons…
s-pic Mar 24, 2020
7d8b2e0
refactor(apiService): cleanup
s-pic Mar 25, 2020
49cb4e6
feat(api.unit.test): test patch shorthand
s-pic Mar 25, 2020
09fb77c
add todo test
s-pic Mar 25, 2020
e970480
fix(api.unit.test.js): syntax error in todo test
s-pic Mar 26, 2020
5e2ac67
refactor(apiService): cleanup
s-pic Mar 31, 2020
86064fd
merge develop
s-pic Mar 31, 2020
d4fde6e
Merge remote-tracking branch 'origin/feature/centralize_api_logic' in…
s-pic Mar 31, 2020
de729e5
Revert "merge develop"
s-pic Mar 31, 2020
c36142a
refactor(api.unit.test.js): cleanup
s-pic Mar 31, 2020
6be8608
Merge branch 'develop' into feature/centralize_api_logic
s-pic Apr 12, 2020
1b0d31c
Fix issue with react-piwik-router in Jest env
cafca Apr 15, 2020
5165dc3
refactor(package.json): remove dotenv from devDependencies
s-pic Apr 22, 2020
9a9fe0a
refactor(utils.js): remove unused isFunction util
s-pic Apr 22, 2020
cd1ddf0
refactor(api.unit.test.js): rename constants
s-pic Apr 22, 2020
7050ffd
refactor(api.unit.test.js): add comment
s-pic Apr 22, 2020
48f6b8a
refactor(apiService): remove unnecessary emptyFunc typing
s-pic Apr 22, 2020
09bd812
refactor(apiService): prettify
s-pic Apr 23, 2020
0fb4e21
refactor(apiService): split into multiple files bundled in index.ts
s-pic Apr 23, 2020
4c463a7
refactor(errorHandling): default export main method to follow lint rules
s-pic Apr 23, 2020
1305a42
improve(errorHandling): if the API answers with a JSON error not hold…
s-pic Apr 23, 2020
655b210
Add review changes
cafca May 6, 2020
84515fb
Simplify constants and helper function
cafca Jul 29, 2020
8f19a5f
Add types, logging
cafca Jul 29, 2020
002b541
Update syntax and add onSlowResponse cb
cafca Jul 29, 2020
ff7de26
Merge branch 'develop' into feature/centralize_api_logic
cafca Jul 29, 2020
a7f5dea
Add docs, rename symbols and modules
cafca Jul 30, 2020
8931e85
refactor(custom errors): remove Error.captureStackTrace() calls
s-pic Aug 4, 2020
c61e3fa
docs(custom errors)
s-pic Aug 4, 2020
6260769
test(api): factor out utils
s-pic Aug 4, 2020
b10f73d
test(api): explicitly test for read- and connection timeout errors
s-pic Aug 4, 2020
ef42349
test(api): test for network errors in a basic (bad) way
s-pic Aug 4, 2020
a100319
test(api): simplify test
s-pic Aug 4, 2020
b527aa8
More concise syntax
cafca Aug 12, 2020
7141027
Set ApiError.message to null if parsing message fails
cafca Aug 12, 2020
82425f1
Revert module name
cafca Aug 12, 2020
69c1863
Update test
cafca Aug 12, 2020
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: 2 additions & 0 deletions jest/__mocks__/ky.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// use ky-universal instead of ky, which is only meant to be used in the browser
module.exports = require('ky-universal');
Comment thread
cafca marked this conversation as resolved.
13 changes: 7 additions & 6 deletions jest/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@

// Some vendors publish their sources without transpiling. You need to say
// jest to transpile such files
const esModules = ['common-tags'];
const esModules = ['common-tags', 'ky'];

module.exports = {
roots: ['<rootDir>/src'],
roots: [
'<rootDir>/src',
'<rootDir>/jest' // allow putting the mocks folder lower down than root https://github.com/facebook/jest/issues/2726#issuecomment-390625860
Comment thread
cafca marked this conversation as resolved.
],
testMatch: [
'**/__tests__/**/*.+(ts|tsx|js)',
'**/?(*.)+(unit|integration).+(spec|test).+(ts|tsx|js)'
Expand Down Expand Up @@ -40,9 +43,7 @@ module.exports = {
'<rootDir>/jest/mocks/fileMock.js',
'images/hbi-stop-icons': '<rootDir>/jest/mocks/fileMock.js',
// handle webpack aliases
'^~/(.*)$': '<rootDir>/src/$1',
// handle ky, see https://github.com/sindresorhus/ky/issues/170
'^ky$': require.resolve('ky').replace('index.js', 'umd.js')
'^~/(.*)$': '<rootDir>/src/$1'
},

// The root directory that Jest should scan for tests and modules within
Expand All @@ -54,5 +55,5 @@ module.exports = {

// An array of regexp pattern strings that are matched against all source
// file paths, matched files will skip transformation
transformIgnorePatterns: [`<rootDir>/node_modules/(?!${esModules})`]
transformIgnorePatterns: [`<rootDir>/node_modules/(?!${esModules.join('|')})`]
};
883 changes: 372 additions & 511 deletions package-lock.json

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"@storybook/react": "^5.3.14",
"@types/fetch-mock": "^7.3.2",
"@types/jest": "^24.9.1",
"@types/node-fetch": "^2.5.5",
"@types/prop-types": "^15.7.3",
"@types/react": "^16.9.32",
"@types/react-redux": "^7.1.7",
Expand Down Expand Up @@ -108,6 +109,7 @@
"@turf/boolean-within": "^6.0.1",
"@turf/helpers": "^6.1.4",
"@turf/length": "^6.0.2",
"@types/debug": "^4.1.5",
"@types/geojson": "^7946.0.7",
"@types/mapbox-gl": "^1.11.1",
"@types/node": "^13.11.0",
Expand All @@ -129,7 +131,7 @@
"iframe-resizer-react": "^1.0.3",
"intersection-observer": "^0.10.0",
"jsonschema": "^1.2.6",
"ky": "^0.17.0",
"ky": "^0.19.0",
"ky-universal": "^0.5.0",
"lodash.debounce": "^4.0.8",
"lodash.isequal": "^4.5.0",
Expand Down
37 changes: 29 additions & 8 deletions src/history.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,40 @@ import { createBrowserHistory } from 'history';
import config from '~/config';

const piwikSiteId = config.piwik.siteId[config.region] || 1;
const piwik = new ReactPiwik({
url: config.piwik.url,
siteId: piwikSiteId,
...config.piwik.options

const browserHistory = createBrowserHistory({
basename: process.env.BASE_NAME
});

const history = piwik.connectToHistory(
createBrowserHistory({ basename: process.env.BASE_NAME })
);
// piwik-react-router has an open issue where it will try and attach a
// script tag in the dom, which doesn't work in a Jest environment.
// As a workaround, the resulting error is caught here.
// See https://github.com/joernroeder/piwik-react-router/issues/48

let history;
try {
const piwik = new ReactPiwik({
url: config.piwik.url,
siteId: piwikSiteId,
...config.piwik.options
});

history = piwik.connectToHistory(browserHistory);
} catch (e) {
if (e.message === "Cannot read property 'parentNode' of undefined") {
history = browserHistory;
} else {
throw e;
}
}

// used in integration tests
if (window.Cypress) {
window.appHistory = history;
}

export default history;
// Export history as a constant, even though it was mutable before
// to handle exception
const constHistory = history;

export default constHistory;
5 changes: 5 additions & 0 deletions src/pages/User/UserState.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,3 +233,8 @@ export default function MapStateReducer(state = initialState, action = {}) {
return state;
}
}

// selectors
export const selectors = {
getToken: (state) => state.UserState.token
};
Comment thread
cafca marked this conversation as resolved.
42 changes: 42 additions & 0 deletions src/services/api/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// TODO: use functions instead of classes to respect eslint, see https://dev.to/damxipo/custom-exceptions-with-js-3aoc

/**
* The api has answered with an error description.
*/
class ApiError extends Error {
public code: number;

constructor(message, statusCode = 500) {
super(message);
this.name = 'ApiError';
this.code = statusCode;
Object.setPrototypeOf(this, ApiError.prototype);
}
}

/**
* Indicates a Read Timeout (it took to long after the last byte has been sent)
* or a Connection Timeout (the server never responded)
*/
class TimeoutError extends Error {
constructor(message) {
super(message);
this.name = 'TimeoutError';
Object.setPrototypeOf(this, TimeoutError.prototype);
}
}

/**
* Communication with the api has failed, usually if
* "when a network error is encountered or CORS is misconfigured on the server-side",
* see https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch
*/
class NetworkError extends Error {
constructor(message) {
super(message);
this.name = 'NetworkError';
Object.setPrototypeOf(this, NetworkError.prototype);
}
}

export { ApiError, TimeoutError, NetworkError };
13 changes: 13 additions & 0 deletions src/services/api/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import request from './request';
import { get, post, patch } from './shorthands';
import { ApiError, TimeoutError, NetworkError } from './errors';

export default {
request,
get,
post,
patch,
ApiError,
TimeoutError,
NetworkError
};
71 changes: 71 additions & 0 deletions src/services/api/mapError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import ky from 'ky-universal';
import debug from 'debug';
import { ApiError, NetworkError, TimeoutError } from './errors';
import { FMCError, JSONValue, JSONObject } from './types';

const log = debug('fmc:api:mapError');

/**
* Translate errors reported by ky/fetch to a set of custom exceptions
* which we can later on use to make decisions on how to handle specific errors.
*/
export default async function mapError(e: FMCError): Promise<FMCError> {
let errorMessage: string;
let statusCode: number;

// handle network errors (misspelled URL, flaky or no network or CORS problems)
if (e.message === 'Failed to fetch') {
return new NetworkError(e.message);
}

// handle all other errors
switch (e.constructor) {
case ky.HTTPError: // a non 2xx error code was found
errorMessage = await parseErrorResponse(e.response);
statusCode = e.response.status;
return new ApiError(errorMessage, statusCode);
case ky.TimeoutError:
return new TimeoutError(e.message);
default:
// any other error, just forward it
return e;
}
}

/**
* Try parsing an error response as JSON or fallback to returning as text
*
* If a JSON error response contains a field `detail` of string type, its
* content is returned as raw text
*
* @param errorResponse API return value
*/
async function parseErrorResponse(
errorResponse: Response
): Promise<string | null> {
let errorJson: JSONValue;
let errorText: string;

// try parse error as text and then as JSON to not rely on content-type
// definitions response headers
try {
errorText = await errorResponse.text();
} catch (e) {
log(e);
throw new TypeError('Only JSON and text error responses can be handled');
}

try {
errorJson = JSON.parse(errorText);
} catch (e) {
log('found error response encoded as raw text');
return errorText;
}

if (typeof (errorJson as JSONObject)?.detail === 'string') {
log('found `detail` field in json response');
return (errorJson as JSONObject)?.detail.toString();
}
log('found json-encoded error response', errorJson);
return null;
}
113 changes: 113 additions & 0 deletions src/services/api/request.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/**
* Facilitates http request creation by
* - offering helper methods with simplified interfaces
* - allowing to hook into before/after a request is made or if an error occurs
* - doing preparatory work for network error request handling
*/
import ky from 'ky-universal';
import debug from 'debug';
import config from '~/config';
import store from '~/store';
import { RequestOptions } from './types';
import { selectors as UserStateSelectors } from '~/pages/User/UserState';
import mapError from './mapError';

const log = debug('fmc:api:request()');

// setup ky

const configuredKy = ky.create({
prefixUrl: config.apiUrl,
hooks: {
beforeRequest: [
// Set authorization headers
(req: Request) => {
const stateRoot = store.getState();
const token = UserStateSelectors.getToken(stateRoot);
if (token) {
req.headers.set('Authorization', `JWT ${token}`);
}
}
]
}
});

// Generic Request Handler

const defaultOptions: RequestOptions = {
accept: 'json',
method: 'get',
timeout: 30 * 1000,
slowResponseTimeout: 5 * 1000
};

/**
* Send requests with convenience callbacks, FMC auth header, raising fmcErrors
Comment thread
s-pic marked this conversation as resolved.
*
* @example
* // Handle server error responses
* try {
* const response = await api.request('https://fixmyberlin.de/api/v1/')
* } catch (err) {
* if(err instanceof api.ApiError) {
* console.log(err.code)
* // 500
* console.log(err.message)
* // parsed from json or text response body
* }
* throw(error)
* }
*
* @example
* // Use callbacks
* await api.request(url, {
* onSubmit: () => dispatch(setLoading(true)),
* onFinish: () => dispatch(setLoading(false)),
* onSlowRequest: () => dispatch(setSlowRequestNotice(true))
* })
*
* @param route URL to request
* @param options object with options, extending ky's options
* @param options.accept expected response body type, either `string` or `json`
* @param options.onSubmit function to call before making request
* @param options.onFinish function to call after request finishes (successful or not)
* @param options.onSlowResponse function to call when request is slow
* @param options.slowResponseTimeout value in milliseconds to wait before
* calling onSlowResponse, defaults to 5000
*/
export default async function request(
route: string,
options: RequestOptions = defaultOptions
): Promise<Response> {
let response;

const {
accept = defaultOptions.accept,
slowResponseTimeout = defaultOptions.slowResponseTimeout,
onSubmit,
onFinish,
Comment thread
s-pic marked this conversation as resolved.
onSlowResponse,
...kyOptions
} = options;

const timeout = setTimeout(() => {
if (onSlowResponse) {
log('calling slow request handler');
onSlowResponse(slowResponseTimeout);
}
}, slowResponseTimeout);

if (onSubmit) onSubmit();
try {
log('sending request', { route, options, accept });
response = await configuredKy(route, kyOptions)[accept]();
} catch (e) {
log('mapping error type', { error: e });
throw await mapError(e);
} finally {
log('finished request');
if (onFinish) onFinish();
clearTimeout(timeout);
}
return response;
}
Loading