From 9d7754e31ee65cd28db4b6580cc8b7f8a9695338 Mon Sep 17 00:00:00 2001 From: Bruce Martin Date: Tue, 30 Oct 2018 19:02:11 -0700 Subject: [PATCH] Improve front-end HTTP error handling (#397) * UI for HTTP error handing in diffexp route * HTTP error handling for other routes * clean up toasters --- client/src/actions/index.js | 138 ++++++++---------- client/src/components/framework/toasters.js | 24 ++- client/src/components/geneExpression/index.js | 17 +-- client/src/util/actionHelpers.js | 22 ++- 4 files changed, 104 insertions(+), 97 deletions(-) diff --git a/client/src/actions/index.js b/client/src/actions/index.js index f82b7e6e..35bf49f1 100644 --- a/client/src/actions/index.js +++ b/client/src/actions/index.js @@ -5,7 +5,8 @@ import { Universe, kvCache } from "../util/stateManager"; import { catchErrorsWrap, doJsonRequest, - rangeEncodeIndices + rangeEncodeIndices, + dispatchNetworkErrorMessageToUser } from "../util/actionHelpers"; /* @@ -53,58 +54,12 @@ const doInitialDataLoad = () => } catch (error) { dispatch({ type: "initial data load error", error }); } - }); - -// XXX TODO - this is the old code for doing a regraph. Preserving it solely -// until we port to 0.2 API. The new UX for regraph can't be implemented on -// the 0.1 API (doesn't allow for re-layout on arbitrary sets of cells), so just -// punting for now. See ticket #88 -// -// -// /* SELECT */ -// const regraph = () => { -// return (dispatch, getState) => { -// dispatch({ type: "regraph started" }); -// -// const state = getState(); -// const selectedMetadata = {}; -// -// _.each(state.controls.categoricalAsBooleansMap, (options, field) => { -// let atLeastOneOptionDeselected = false; -// -// _.each(options, (isActive, option) => { -// if (!isActive) { -// atLeastOneOptionDeselected = true; -// } -// }); -// -// if (atLeastOneOptionDeselected) { -// _.each(options, (isActive, option) => { -// if (isActive) { -// if (selectedMetadata[field]) { -// selectedMetadata[field].push(option); -// } else if (!selectedMetadata[field]) { -// selectedMetadata[field] = [option]; -// } -// } -// }); -// } -// }); -// -// let uri = new URI(); -// uri.setSearch(selectedMetadata); -// console.log(uri.search(), selectedMetadata); -// -// dispatch(requestCells(uri.search())).then(res => { -// if (res.error) { -// dispatch({ type: "regraph error" }); -// } else { -// dispatch({ type: "regraph success" }); -// } -// }); -// }; -// }; + }, true); +/* +Set the view (world) to current selection. Placeholder for an async action +which also does re-layout. +*/ const regraph = () => (dispatch, getState) => { const { universe, world, crossfilter } = getState().controls; dispatch({ @@ -115,6 +70,15 @@ const regraph = () => (dispatch, getState) => { }); }; +// Throws +const dispatchExpressionErrors = (dispatch, res) => { + const msg = `Unexpected HTTP response while fetching expression data ${ + res.status + }, ${res.statusText}`; + dispatchNetworkErrorMessageToUser(msg); + throw new Error(msg); +}; + /* Fetch expression vectors for each gene in genes. This is NOT an action function, but rather a helper to be called from an action helper that @@ -141,8 +105,7 @@ async function _doRequestExpressionData(dispatch, getState, genes) { if (genesToFetch.length) { try { // XXX: TODO - this could be using /data/var rather than /data/obs, - // as that would simplify the transformation in - // convertExpressionRESTv02ToObject + // as that would simplify the transformation in convertExpressionRESTv02ToObject const res = await fetch( `${globals.API.prefix}${globals.API.version}data/obs`, { @@ -161,6 +124,12 @@ async function _doRequestExpressionData(dispatch, getState, genes) { }) } ); + + if (!res.ok || res.headers.get("Content-Type") !== "application/json") { + // WILL throw + return dispatchExpressionErrors(dispatch, res); + } + const data = await res.json(); expressionData = { ...expressionData, @@ -201,28 +170,12 @@ function requestSingleGeneExpressionCountsForColoringPOST(gene) { }; } -const requestGeneExpressionCountsPOST = genes => async (dispatch, getState) => { - dispatch({ type: "get expression started" }); - try { - const expressionData = await _doRequestExpressionData( - dispatch, - getState, - genes - ); - return dispatch({ - type: "get expression success", - genes, - data: expressionData - }); - } catch (error) { - return dispatch({ type: "get expression error", error }); - } -}; - -const requestUserDefinedGene = gene => async dispatch => { +const requestUserDefinedGene = gene => async (dispatch, getState) => { dispatch({ type: "request user defined gene started" }); try { - const data = await dispatch(requestGeneExpressionCountsPOST([gene])); + const data = await await _doRequestExpressionData(dispatch, getState, [ + gene + ]); /* then send the success case action through */ return dispatch({ @@ -237,6 +190,31 @@ const requestUserDefinedGene = gene => async dispatch => { } }; +const dispatchDiffExpErrors = (dispatch, response) => { + switch (response.status) { + case 403: + dispatchNetworkErrorMessageToUser( + "Too many cells selected for differential experesion calculation - please make a smaller selection." + ); + break; + case 501: + dispatchNetworkErrorMessageToUser( + "Differential expression is not implemented." + ); + break; + default: { + const msg = `Unexpected differential expression HTTP response ${ + response.status + }, ${response.statusText}`; + dispatchNetworkErrorMessageToUser(msg); + dispatch({ + type: "request differential expression error", + error: new Error(msg) + }); + } + } +}; + const requestDifferentialExpression = (set1, set2, num_genes = 10) => async ( dispatch, getState @@ -256,7 +234,7 @@ const requestDifferentialExpression = (set1, set2, num_genes = 10) => async ( const set2ByIndex = rangeEncodeIndices( _.map(set2, s => universe.obsNameToIndexMap[s]) ); - const diffExpFetch = await fetch( + const res = await fetch( `${globals.API.prefix}${globals.API.version}diffexp/obs`, { method: "POST", @@ -273,7 +251,12 @@ const requestDifferentialExpression = (set1, set2, num_genes = 10) => async ( }) } ); - const data = await diffExpFetch.json(); + + if (!res.ok || res.headers.get("Content-Type") !== "application/json") { + return dispatchDiffExpErrors(dispatch, res); + } + + const data = await res.json(); // result is [ [varIdx, ...], ... ] const topNGenes = _.map(data, r => universe.varAnnotations[r[0]].name); @@ -281,7 +264,7 @@ const requestDifferentialExpression = (set1, set2, num_genes = 10) => async ( Kick off secondary action to fetch all of the expression data for the topN expressed genes. */ - await dispatch(requestGeneExpressionCountsPOST(topNGenes)); + await _doRequestExpressionData(dispatch, getState, topNGenes); /* then send the success case action through */ return dispatch({ @@ -321,7 +304,6 @@ export default { regraph, resetInterface, requestSingleGeneExpressionCountsForColoringPOST, - requestGeneExpressionCountsPOST, requestDifferentialExpression, requestUserDefinedGene, doInitialDataLoad diff --git a/client/src/components/framework/toasters.js b/client/src/components/framework/toasters.js index 1a8237dc..3b0e67f4 100644 --- a/client/src/components/framework/toasters.js +++ b/client/src/components/framework/toasters.js @@ -1,12 +1,24 @@ import { Position, Toaster, Intent } from "@blueprintjs/core"; /** Singleton toaster instance. Create separate instances for different options. */ -export const ErrorToastTopCenter = Toaster.create({ + +const ErrorToastTopCenter = Toaster.create({ className: "recipe-toaster", - position: Position.TOP, - intent: Intent.WARNING + position: Position.TOP }); -export default { - ErrorToastTopCenter -}; +/* +A "user" error - eg, bad input +*/ +export const postUserErrorToast = message => + ErrorToastTopCenter.show({ message, intent: Intent.WARNING }); + +/* +a hard network error +*/ +export const postNetworkErrorToast = message => + ErrorToastTopCenter.show({ + message, + timeout: 30000, + intent: Intent.DANGER + }); diff --git a/client/src/components/geneExpression/index.js b/client/src/components/geneExpression/index.js index 03f7c113..04f6b871 100644 --- a/client/src/components/geneExpression/index.js +++ b/client/src/components/geneExpression/index.js @@ -9,7 +9,7 @@ import { Button, Tooltip } from "@blueprintjs/core"; import HistogramBrush from "../brushableHistogram"; import * as globals from "../../globals"; import actions from "../../actions"; -import { ErrorToastTopCenter } from "../framework/toasters"; +import { postUserErrorToast } from "../framework/toasters"; import ExpressionButtons from "./expressionButtons"; @connect(state => { @@ -47,18 +47,13 @@ class GeneExpression extends React.Component { const { gene } = this.state; if (userDefinedGenes.indexOf(gene) !== -1) { - ErrorToastTopCenter.show({ - message: "That gene already exists" - }); + postUserErrorToast("That gene already exists"); } else if (userDefinedGenes.length > 15) { - ErrorToastTopCenter.show({ - message: - "That's too many genes, you can have at most 15 user defined genes" - }); + postUserErrorToast( + "That's too many genes, you can have at most 15 user defined genes" + ); } else if (!_.find(world.varAnnotations, { name: gene })) { - ErrorToastTopCenter.show({ - message: "That doesn't appear to be a valid gene name." - }); + postUserErrorToast("That doesn't appear to be a valid gene name."); } else { dispatch(actions.requestUserDefinedGene(gene)); dispatch({ diff --git a/client/src/util/actionHelpers.js b/client/src/util/actionHelpers.js index cf408b05..7ee68c59 100644 --- a/client/src/util/actionHelpers.js +++ b/client/src/util/actionHelpers.js @@ -1,12 +1,24 @@ import _ from "lodash"; +/* XXX: cough, cough, ... */ +import { postNetworkErrorToast } from "../components/framework/toasters"; + +/* +dispatch an action error to the user. Currently we use +async toasts. +*/ +export const dispatchNetworkErrorMessageToUser = message => + postNetworkErrorToast(message); /* Catch unexpected errors and make sure we don't lose them! */ -export function catchErrorsWrap(fn) { +export function catchErrorsWrap(fn, dispatchToUser = false) { return (dispatch, getState) => { fn(dispatch, getState).catch(error => { console.error(error); + if (dispatchToUser) { + dispatchNetworkErrorMessageToUser(error.message); + } dispatch({ type: "UNEXPECTED ERROR", error }); }); }; @@ -23,7 +35,13 @@ export const doJsonRequest = async url => { "Accept-Encoding": "gzip, deflate, br" }) }); - return res.json(); + if (res.ok && res.headers.get("Content-Type") === "application/json") { + return res.json(); + } + // else an error + const msg = `Unexpected HTTP response ${res.status}, ${res.statusText}`; + dispatchNetworkErrorMessageToUser(msg); + throw new Error(msg); }; /*