From 97791c573abc2a2b2f4eeca7fb4f44e310a1ac71 Mon Sep 17 00:00:00 2001 From: Sanjeev Lakhwani Date: Tue, 4 Jul 2023 09:51:11 +0530 Subject: [PATCH 1/3] Refactor error handling in network fetch functions --- src/utils/actions.js | 146 +++++++++++++++++++++++-------------------- 1 file changed, 77 insertions(+), 69 deletions(-) diff --git a/src/utils/actions.js b/src/utils/actions.js index 6684ad2ff..d1c4381b5 100644 --- a/src/utils/actions.js +++ b/src/utils/actions.js @@ -1,29 +1,29 @@ import fetch from "cross-fetch"; -import {message} from "antd"; +import { message } from "antd"; -import {BENTO_PUBLIC_URL, BENTO_URL, IDP_BASE_URL} from "../config"; +import { BENTO_PUBLIC_URL, BENTO_URL, IDP_BASE_URL } from "../config"; -export const basicAction = t => () => ({type: t}); +export const basicAction = (t) => () => ({ type: t }); -export const createNetworkActionTypes = name => ({ +export const createNetworkActionTypes = (name) => ({ REQUEST: `${name}.REQUEST`, RECEIVE: `${name}.RECEIVE`, ERROR: `${name}.ERROR`, FINISH: `${name}.FINISH`, }); -export const createFlowActionTypes = name => ({ +export const createFlowActionTypes = (name) => ({ BEGIN: `${name}.BEGIN`, END: `${name}.END`, TERMINATE: `${name}.TERMINATE`, }); - const _unpaginatedNetworkFetch = async (url, _baseUrl, req, parse) => { const response = await fetch(url, req); if (!response.ok) { - throw `${response.status} ${response.statusText}`; + const errorData = await parse(response); + throw new Error(errorData.message || `${response.status} ${response.statusText}`); } return response.status === 204 ? null : await parse(response); }; @@ -33,13 +33,14 @@ const _paginatedNetworkFetch = async (url, baseUrl, req, parse) => { const _fetchNext = async (pageUrl) => { const response = await fetch(pageUrl, req); if (!response.ok) { - throw "Invalid response encountered"; + const errorData = await parse(response); + throw new Error(errorData.message || "Invalid response encountered"); } const data = await parse(response); if (!data.hasOwnProperty("results")) throw "Missing results set"; const pageResults = data.results; - const nextUrl = data.next ? (baseUrl + data.next) : null; + const nextUrl = data.next ? baseUrl + data.next : null; if (!(pageResults instanceof Array)) throw "Invalid results set"; results.push(...pageResults); if (nextUrl) await _fetchNext(nextUrl); @@ -48,65 +49,72 @@ const _paginatedNetworkFetch = async (url, baseUrl, req, parse) => { return results; }; - -const _networkAction = (fn, ...args) => async (dispatch, getState) => { - let fnResult = await fn(...args); - if (typeof fnResult === "function") { - // Needs dispatch / getState, resolve those. - fnResult = await fnResult(dispatch, getState); - } - - const {types, params, url, baseUrl, req, err, onSuccess, onError, paginated} = fnResult; - - // Only include access token when we are making a request to this Bento node or the IdP! - // Otherwise, we could leak it to external sites. - - const token = ( - url.startsWith("/") || - (BENTO_URL && url.startsWith(BENTO_URL)) || - (BENTO_PUBLIC_URL && url.startsWith(BENTO_PUBLIC_URL)) || - (IDP_BASE_URL && url.startsWith(IDP_BASE_URL)) - ) ? getState().auth.accessToken : null; - - const finalReq = { - ...(req ?? { - method: "GET", // Default request method - }), - headers: { - ...(token ? {"Authorization": `Bearer ${token}`} : {}), - ...(req?.headers ?? {}), - }, - }; - - let {parse} = fnResult; - if (!parse) parse = r => r.json(); - - dispatch({type: types.REQUEST, ...params}); - try { - const data = await (paginated ? _paginatedNetworkFetch : _unpaginatedNetworkFetch)( - url, baseUrl, finalReq, parse); - dispatch({ - type: types.RECEIVE, - ...params, - ...(data === null ? {} : {data}), - receivedAt: Date.now(), - }); - if (onSuccess) await onSuccess(data); - } catch (e) { - if (err) { - console.error(e, err); - message.error(err); - } - dispatch({type: types.ERROR, ...params, caughtError: e}); - if (onError) await onError(e); - } - dispatch({type: types.FINISH, ...params}); -}; +const _networkAction = + (fn, ...args) => + async (dispatch, getState) => { + let fnResult = await fn(...args); + if (typeof fnResult === "function") { + // Needs dispatch / getState, resolve those. + fnResult = await fnResult(dispatch, getState); + } + + const { types, params, url, baseUrl, req, err, onSuccess, onError, paginated } = fnResult; + + // Only include access token when we are making a request to this Bento node or the IdP! + // Otherwise, we could leak it to external sites. + + const token = + url.startsWith("/") || + (BENTO_URL && url.startsWith(BENTO_URL)) || + (BENTO_PUBLIC_URL && url.startsWith(BENTO_PUBLIC_URL)) || + (IDP_BASE_URL && url.startsWith(IDP_BASE_URL)) + ? getState().auth.accessToken + : null; + + const finalReq = { + ...(req ?? { + method: "GET", // Default request method + }), + headers: { + ...(token ? { Authorization: `Bearer ${token}` } : {}), + ...(req?.headers ?? {}), + }, + }; + + let { parse } = fnResult; + if (!parse) parse = (r) => r.json(); + + dispatch({ type: types.REQUEST, ...params }); + try { + const data = await (paginated ? _paginatedNetworkFetch : _unpaginatedNetworkFetch)( + url, + baseUrl, + finalReq, + parse, + ); + dispatch({ + type: types.RECEIVE, + ...params, + ...(data === null ? {} : { data }), + receivedAt: Date.now(), + }); + if (onSuccess) await onSuccess(data); + } catch (e) { + const errorMsg = e.message || err; + console.error(e, errorMsg); + message.error(errorMsg); + dispatch({ type: types.ERROR, ...params, caughtError: e }); + if (onError) await onError(e); + } + dispatch({ type: types.FINISH, ...params }); + }; // Curried version -export const networkAction = fn => (...args) => _networkAction(fn, ...args); - - -export const beginFlow = types => async dispatch => await dispatch({type: types.BEGIN}); -export const endFlow = types => async dispatch => await dispatch({type: types.END}); -export const terminateFlow = types => async dispatch => await dispatch({type: types.TERMINATE}); +export const networkAction = + (fn) => + (...args) => + _networkAction(fn, ...args); + +export const beginFlow = (types) => async (dispatch) => await dispatch({ type: types.BEGIN }); +export const endFlow = (types) => async (dispatch) => await dispatch({ type: types.END }); +export const terminateFlow = (types) => async (dispatch) => await dispatch({ type: types.TERMINATE }); From 43d9c75fdbfbad234b93d0ef51a21d15afaf07b3 Mon Sep 17 00:00:00 2001 From: Sanjeev Lakhwani Date: Mon, 10 Jul 2023 08:07:19 +0530 Subject: [PATCH 2/3] resolved PR issues --- src/utils/actions.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/utils/actions.js b/src/utils/actions.js index d1c4381b5..bfa4c8964 100644 --- a/src/utils/actions.js +++ b/src/utils/actions.js @@ -33,8 +33,12 @@ const _paginatedNetworkFetch = async (url, baseUrl, req, parse) => { const _fetchNext = async (pageUrl) => { const response = await fetch(pageUrl, req); if (!response.ok) { - const errorData = await parse(response); - throw new Error(errorData.message || "Invalid response encountered"); + try { + const errorData = await parse(response); + throw new Error(errorData.message); + } catch (_) { + throw new Error("Invalid response encountered"); + } } const data = await parse(response); @@ -100,7 +104,7 @@ const _networkAction = }); if (onSuccess) await onSuccess(data); } catch (e) { - const errorMsg = e.message || err; + const errorMsg = err + (e.message ? `: ${e.message}` : ""); console.error(e, errorMsg); message.error(errorMsg); dispatch({ type: types.ERROR, ...params, caughtError: e }); From cadbfb5c28595e8357d74c92f6d81a4b10d01d23 Mon Sep 17 00:00:00 2001 From: Sanjeev Lakhwani Date: Tue, 1 Aug 2023 12:39:16 -0400 Subject: [PATCH 3/3] PR changes --- src/utils/actions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/actions.js b/src/utils/actions.js index bfa4c8964..e2ae3063a 100644 --- a/src/utils/actions.js +++ b/src/utils/actions.js @@ -104,7 +104,7 @@ const _networkAction = }); if (onSuccess) await onSuccess(data); } catch (e) { - const errorMsg = err + (e.message ? `: ${e.message}` : ""); + const errorMsg = err ? (err + (e.message ? `: ${e.message}` : "") ) : e.message; console.error(e, errorMsg); message.error(errorMsg); dispatch({ type: types.ERROR, ...params, caughtError: e });