tighten up error reporting (#1269)

* black reformat

* tighten up error reporting

* lint

* fine tuning

* additional improvements in exception handling

* lint

* include exception and traceback in log

* fix typo
This commit is contained in:
Bruce Martin
2020-03-22 09:55:47 -07:00
committed by GitHub
parent 8180be83b8
commit db7a485796
9 changed files with 124 additions and 129 deletions
+29 -27
View File
@@ -1,5 +1,6 @@
import os
import datetime
import logging
from flask import Flask, redirect, current_app, make_response, render_template, abort
from flask import Blueprint, request, send_from_directory
@@ -40,8 +41,10 @@ def dataset_index(dataset=None):
with cache_manager.data_adaptor(location, config) as data_adaptor:
dataset_title = config.get_title(data_adaptor)
return render_template("index.html", datasetTitle=dataset_title, SCRIPTS=scripts)
except DatasetAccessError as e:
return make_response(f"Invalid dataset {dataset}: {str(e)}", HTTPStatus.BAD_REQUEST)
except DatasetAccessError:
return common_rest.abort_and_log(
HTTPStatus.BAD_REQUEST, f"Invalid dataset {dataset}", loglevel=logging.INFO, include_exc_info=True
)
@webbp.route("/favicon.png", methods=["GET"])
@@ -69,7 +72,7 @@ def get_data_adaptor(dataset=None):
raise DatasetAccessError("Invalid dataset {dataset}")
if datapath is None:
return make_response("Dataset must be supplied", HTTPStatus.BAD_REQUEST)
return common_rest.abort_and_log(HTTPStatus.BAD_REQUEST, f"Invalid dataset NONE", loglevel=logging.INFO)
cache_manager = current_app.matrix_data_cache_manager
return cache_manager.data_adaptor(datapath, config)
@@ -81,8 +84,10 @@ def rest_get_data_adaptor(func):
try:
with get_data_adaptor(dataset) as data_adaptor:
return func(self, data_adaptor)
except DatasetAccessError as e:
return make_response(f"Invalid dataset {dataset}: {str(e)}", HTTPStatus.BAD_REQUEST)
except DatasetAccessError:
return common_rest.abort_and_log(
HTTPStatus.BAD_REQUEST, f"Invalid dataset {dataset}", loglevel=logging.INFO, include_exc_info=True
)
return wrapped_function
@@ -98,29 +103,26 @@ def dataroot_test_index():
data += "<head><title>Hosted Cellxgene</title></head>"
data += "<body><H1>Welcome to cellxgene</H1>"
try:
config = current_app.app_config
locator = DataLocator(config.multi_dataset__dataroot)
datasets = []
for fname in locator.ls():
location = path_join(config.multi_dataset__dataroot, fname)
try:
MatrixDataLoader(location, app_config=config)
datasets.append(fname)
except DatasetAccessError:
# skip over invalid datasets
pass
data += "<br/>Select one of these datasets...<br/>"
data += "<ul>"
datasets.sort()
for dataset in datasets:
data += f"<li><a href={dataset}>{dataset}</a></li>"
data += "</ul>"
except Exception as e:
data += f'<br/>Unable to locate datasets from {config.multi_dataset__dataroot}: {str(e)}'
config = current_app.app_config
locator = DataLocator(config.multi_dataset__dataroot)
datasets = []
for fname in locator.ls():
location = path_join(config.multi_dataset__dataroot, fname)
try:
MatrixDataLoader(location, app_config=config)
datasets.append(fname)
except DatasetAccessError:
# skip over invalid datasets
pass
data += "<br/>Select one of these datasets...<br/>"
data += "<ul>"
datasets.sort()
for dataset in datasets:
data += f"<li><a href={dataset}>{dataset}</a></li>"
data += "</ul>"
data += "</body></html>"
return make_response(data)
@@ -128,7 +130,7 @@ def dataroot_index():
# Handle the base url for the cellxgene server when running in multi dataset mode
config = current_app.app_config
if not config.multi_dataset__index:
abort(404)
abort(HTTPStatus.NOT_FOUND)
elif config.multi_dataset__index is True:
return dataroot_test_index()
else:
+6 -7
View File
@@ -11,7 +11,6 @@ from server.common.errors import AnnotationsError, OntologyLoadFailure
from server.common.utils import series_to_schema
import fsspec
import fastobo
import traceback # use built-in formatter for SyntaxError
from flask import session
from abc import ABCMeta, abstractmethod
@@ -39,14 +38,13 @@ class Annotations(metaclass=ABCMeta):
self.ontology_data = names
except FileNotFoundError as e:
raise OntologyLoadFailure(f"Unable to find OBO ontology path: {path}") from e
raise OntologyLoadFailure("Unable to find OBO ontology path") from e
except SyntaxError as e:
msg = "".join(traceback.format_exception_only(SyntaxError, e))
raise OntologyLoadFailure(msg) from e
raise OntologyLoadFailure("Syntax error loading OBO ontology") from e
except Exception as e:
raise OntologyLoadFailure(f"Error loading OBO file {path}") from e
raise OntologyLoadFailure("Error loading OBO file") from e
def get_schema(self, data_adaptor):
labels = self.read_labels(data_adaptor)
@@ -124,8 +122,9 @@ class AnnotationsLocalFile(Annotations):
if fname == self.last_fname:
return self.last_labels
else:
labels = pd.read_csv(fname, dtype="category",
index_col=0, header=0, comment="#", keep_default_na=False)
labels = pd.read_csv(
fname, dtype="category", index_col=0, header=0, comment="#", keep_default_na=False
)
# update the cache
self.last_fname = fname
self.last_labels = labels
+63 -69
View File
@@ -1,7 +1,8 @@
import sys
from http import HTTPStatus
import warnings
import copy
from flask import make_response, jsonify
import logging
from flask import make_response, jsonify, current_app, abort
from server.common.constants import Axis, DiffExpMode, JSON_NaN_to_num_warning_msg
from server.common.errors import (
FilterError,
@@ -14,6 +15,20 @@ import json
from server.data_common.fbs.matrix import decode_matrix_fbs
def abort_and_log(code, logmsg, loglevel=logging.DEBUG, include_exc_info=False):
"""
Log the message, then abort with HTTP code. If include_exc_info is true,
also include current exception via sys.exc_info().
"""
if include_exc_info:
exc_info = sys.exc_info()
else:
exc_info = False
current_app.logger.log(loglevel, logmsg, exc_info=exc_info)
# Do NOT send log message to HTTP response.
return abort(code)
def schema_get_helper(data_adaptor, annotations):
"""helper function to gather the schema from the data source and annotations"""
schema = data_adaptor.get_schema()
@@ -41,19 +56,15 @@ def annotations_obs_get(request, data_adaptor, annotations):
fields = request.args.getlist("annotation-name", None)
preferred_mimetype = request.accept_mimetypes.best_match(["application/octet-stream"])
if preferred_mimetype != "application/octet-stream":
return make_response(f"Unsupported MIME type '{request.accept_mimetypes}'", HTTPStatus.NOT_ACCEPTABLE)
return abort(HTTPStatus.NOT_ACCEPTABLE)
try:
labels = None
if annotations:
labels = annotations.read_labels(data_adaptor)
fbs = data_adaptor.annotation_to_fbs_matrix(Axis.OBS, fields, labels)
return make_response(fbs, HTTPStatus.OK, {"Content-Type": "application/octet-stream"})
except KeyError:
return make_response(f"Error bad key in {fields}", HTTPStatus.BAD_REQUEST)
except ValueError as e:
return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
except Exception as e:
return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
except KeyError as e:
return abort_and_log(HTTPStatus.BAD_REQUEST, str(e), include_exc_info=True)
def annotations_put_fbs_helper(data_adaptor, annotations, fbs):
@@ -69,14 +80,14 @@ def annotations_put_fbs_helper(data_adaptor, annotations, fbs):
def annotations_obs_put(request, data_adaptor, annotations):
if annotations is None:
return make_response("Error, annotations are not configured", HTTPStatus.BAD_REQUEST)
return abort(HTTPStatus.NOT_IMPLEMENTED)
anno_collection = request.args.get("annotation-collection-name", default=None)
fbs = request.get_data()
if anno_collection is not None:
if not annotations.is_safe_collection_name(anno_collection):
return make_response(f"Error, bad annotation collection name", HTTPStatus.BAD_REQUEST)
return abort(HTTPStatus.BAD_REQUEST, "Bad annotation collection name")
annotations.set_collection(anno_collection)
try:
@@ -84,16 +95,14 @@ def annotations_obs_put(request, data_adaptor, annotations):
res = json.dumps({"status": "OK"})
return make_response(res, HTTPStatus.OK, {"Content-Type": "application/json"})
except (ValueError, DisabledFeatureError, KeyError) as e:
return make_response(str(e), HTTPStatus.BAD_REQUEST)
except Exception as e:
return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
return abort_and_log(HTTPStatus.BAD_REQUEST, str(e), include_exc_info=True)
def annotations_var_get(request, data_adaptor, annotations):
fields = request.args.getlist("annotation-name", None)
preferred_mimetype = request.accept_mimetypes.best_match(["application/octet-stream"])
if preferred_mimetype != "application/octet-stream":
return make_response(f"Unsupported MIME type '{request.accept_mimetypes}'", HTTPStatus.NOT_ACCEPTABLE)
return abort(HTTPStatus.NOT_ACCEPTABLE)
try:
labels = None
if annotations is not None:
@@ -103,18 +112,14 @@ def annotations_var_get(request, data_adaptor, annotations):
HTTPStatus.OK,
{"Content-Type": "application/octet-stream"},
)
except KeyError:
return make_response(f"Error bad key in {fields}", HTTPStatus.BAD_REQUEST)
except ValueError as e:
return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
except Exception as e:
return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
except KeyError as e:
return abort_and_log(HTTPStatus.BAD_REQUEST, str(e), include_exc_info=True)
def data_var_put(request, data_adaptor):
preferred_mimetype = request.accept_mimetypes.best_match(["application/octet-stream"])
if preferred_mimetype != "application/octet-stream":
return make_response(f"Unsupported MIME type '{request.accept_mimetypes}'", HTTPStatus.NOT_ACCEPTABLE)
return abort(HTTPStatus.NOT_ACCEPTABLE)
filter_json = request.get_json()
filter = filter_json["filter"] if filter_json else None
@@ -125,58 +130,43 @@ def data_var_put(request, data_adaptor):
{"Content-Type": "application/octet-stream"},
)
except FilterError as e:
return make_response(str(e), HTTPStatus.BAD_REQUEST)
except ValueError as e:
return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
return abort_and_log(HTTPStatus.BAD_REQUEST, str(e), include_exc_info=True)
def diffexp_obs_post(request, data_adaptor):
if not data_adaptor.config.diffexp__enable:
return make_response(f"diffexp not supported.", HTTPStatus.BAD_REQUEST)
return abort(HTTPStatus.NOT_IMPLEMENTED)
args = request.get_json()
# confirm mode is present and legal
try:
# TODO: implement varfilter mode
mode = DiffExpMode(args["mode"])
except (KeyError, TypeError):
return make_response("Error: mode is required", HTTPStatus.BAD_REQUEST)
except ValueError:
return make_response(f"Error: invalid mode option {args['mode']}", HTTPStatus.BAD_REQUEST)
# Validate filters
if mode == DiffExpMode.VAR_FILTER or "varFilter" in args:
# not NOT_IMPLEMENTED
return make_response("mode=varfilter not implemented", HTTPStatus.NOT_IMPLEMENTED)
if mode == DiffExpMode.TOP_N and "count" not in args:
return make_response("mode=topN requires a count parameter", HTTPStatus.BAD_REQUEST)
if "set1" not in args:
return make_response("set1 is required.", HTTPStatus.BAD_REQUEST)
if Axis.VAR in args["set1"]["filter"]:
return make_response("Var filter not allowed for set1", HTTPStatus.BAD_REQUEST)
# set2
if "set2" not in args:
return make_response("Set2 as inverse of set1 is not implemented", HTTPStatus.NOT_IMPLEMENTED)
if Axis.VAR in args["set2"]["filter"]:
return make_response("Var filter not allowed for set2", HTTPStatus.BAD_REQUEST)
if mode == DiffExpMode.VAR_FILTER or "varFilter" in args:
return abort_and_log(HTTPStatus.NOT_IMPLEMENTED, "varFilter not enabled")
set1_filter = args["set1"]["filter"]
set2_filter = args.get("set2", {"filter": {}})["filter"]
set1_filter = args.get("set1", {"filter": {}})["filter"]
set2_filter = args.get("set2", {"filter": {}})["filter"]
count = args.get("count", None)
# TODO: implement varfilter mode
if set1_filter is None or set2_filter is None or count is None:
return abort_and_log(HTTPStatus.BAD_REQUEST, "missing required parameter")
if Axis.VAR in set1_filter or Axis.VAR in set2_filter:
return abort_and_log(HTTPStatus.BAD_REQUEST, "var axis filter not enabled")
except (KeyError, TypeError) as e:
return abort_and_log(HTTPStatus.BAD_REQUEST, str(e), include_exc_info=True)
# mode=topN
count = args.get("count", None)
try:
diffexp = data_adaptor.diffexp_topN(set1_filter, set2_filter, count)
return make_response(diffexp, HTTPStatus.OK, {"Content-Type": "application/json"})
except (ValueError, DisabledFeatureError, FilterError) as e:
return make_response(str(e), HTTPStatus.BAD_REQUEST)
except JSONEncodingValueError as e:
# JSON encoding failure, usually due to bad data
warnings.warn(JSON_NaN_to_num_warning_msg)
return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
except ValueError as e:
return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
return abort_and_log(HTTPStatus.BAD_REQUEST, str(e), include_exc_info=True)
except JSONEncodingValueError:
# JSON encoding failure, usually due to bad data. Just let it ripple up
# to default exception handler.
current_app.logger.warning(JSON_NaN_to_num_warning_msg)
raise
def layout_obs_get(request, data_adaptor):
@@ -187,24 +177,28 @@ def layout_obs_get(request, data_adaptor):
data_adaptor.layout_to_fbs_matrix(), HTTPStatus.OK, {"Content-Type": "application/octet-stream"}
)
else:
return make_response(f"Unsupported MIME type '{request.accept_mimetypes}'", HTTPStatus.NOT_ACCEPTABLE)
except PrepareError as e:
return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
except ValueError as e:
return make_response(str(e), HTTPStatus.INTERNAL_SERVER_ERROR)
return abort(HTTPStatus.NOT_ACCEPTABLE)
except PrepareError:
return abort_and_log(
HTTPStatus.NOT_IMPLEMENTED,
f"No embedding available {request.path}",
loglevel=logging.ERROR,
include_exc_info=True,
)
def layout_obs_put(request, data_adaptor):
if not data_adaptor.config.embedding__enable_reembedding:
return abort(HTTPStatus.NOT_IMPLEMENTED)
preferred_mimetype = request.accept_mimetypes.best_match(["application/octet-stream"])
if preferred_mimetype != "application/octet-stream":
return make_response(f"Unsupported MIME type '{request.accept_mimetypes}'", HTTPStatus.NOT_ACCEPTABLE)
if not data_adaptor.config.embedding__enable_reembedding:
return make_response(f"Computed embedding not supported.", HTTPStatus.BAD_REQUEST)
return abort(HTTPStatus.NOT_ACCEPTABLE)
args = request.get_json()
filter = args["filter"] if args else None
if not filter:
return make_response("Error: obs filter is required", HTTPStatus.BAD_REQUEST)
return abort_and_log(HTTPStatus.BAD_REQUEST, "obs filter is required")
method = args["method"] if args else "umap"
try:
@@ -219,6 +213,6 @@ def layout_obs_put(request, data_adaptor):
},
)
except NotImplementedError as e:
return make_response(str(e), HTTPStatus.NOT_IMPLEMENTED)
return abort_and_log(HTTPStatus.NOT_IMPLEMENTED, str(e), include_exc_info=True)
except (ValueError, DisabledFeatureError, FilterError) as e:
return make_response(str(e), HTTPStatus.BAD_REQUEST)
return abort_and_log(HTTPStatus.BAD_REQUEST, str(e), include_exc_info=True)
+1 -1
View File
@@ -15,7 +15,7 @@ def get_scanpy_module():
raise NotImplementedError("Please install scanpy to enable UMAP re-embedding")
except Exception as e:
# will capture other ImportError corner cases
raise NotImplementedError(str(e))
raise NotImplementedError() from e
def scanpy_umap(adata, obs_mask=None, pca_options={}, neighbors_options={}, umap_options={}):
+7 -8
View File
@@ -44,9 +44,9 @@ class AnndataAdaptor(DataAdaptor):
# try to read it. Many of these tests don't make sense for URIs (eg, extension-
# based typing).
if not data_locator.exists():
raise DatasetAccessError(f"{data_locator.uri_or_path} does not exist")
raise DatasetAccessError("does not exist")
if not data_locator.isfile():
raise DatasetAccessError(f"{data_locator.uri_or_path} is not a file")
raise DatasetAccessError("is not a file")
@staticmethod
def file_size(data_locator):
@@ -172,10 +172,10 @@ class AnndataAdaptor(DataAdaptor):
)
except MemoryError:
raise DatasetAccessError("Out of memory - file is too large for available memory.")
except Exception as e:
except Exception:
raise DatasetAccessError(
f"{e} - file not found or is inaccessible. File must be an .h5ad object. "
f"Please check your input and try again."
"File not found or is inaccessible. File must be an .h5ad object. "
"Please check your input and try again."
)
def _validate_and_initialize(self):
@@ -310,9 +310,8 @@ class AnndataAdaptor(DataAdaptor):
try:
shape = self.get_shape()
obs_mask = self._axis_filter_to_mask(Axis.OBS, obsFilter["obs"], shape[0])
except (KeyError, IndexError) as e:
raise FilterError(f"Error parsing filter: {e}") from e
except (KeyError, IndexError):
raise FilterError("Error parsing filter")
with ServerTiming.time("layout.compute"):
X_umap = scanpy_umap(self.data, obs_mask)
normalized_layout = DataAdaptor.normalize_embedding(X_umap)
+4 -4
View File
@@ -253,8 +253,8 @@ class DataAdaptor(metaclass=ABCMeta):
try:
obs_selector, var_selector = self._filter_to_mask(filter)
except (KeyError, IndexError, TypeError, AttributeError) as e:
raise FilterError(f"Error parsing filter: {e}") from e
except (KeyError, IndexError, TypeError, AttributeError):
raise FilterError("Error parsing filter")
if obs_selector is not None:
raise FilterError("filtering on obs unsupported")
@@ -281,8 +281,8 @@ class DataAdaptor(metaclass=ABCMeta):
shape = self.get_shape()
obs_mask_A = self._axis_filter_to_mask(Axis.OBS, obsFilterA["obs"], shape[0])
obs_mask_B = self._axis_filter_to_mask(Axis.OBS, obsFilterB["obs"], shape[0])
except (KeyError, IndexError) as e:
raise FilterError(f"Error parsing filter: {e}") from e
except (KeyError, IndexError):
raise FilterError("Error parsing filter")
if top_n is None:
top_n = DEFAULT_TOP_N
+1 -2
View File
@@ -148,8 +148,7 @@ class MatrixDataLoader(object):
self.matrix_data_type = self.__matrix_data_type()
if not self.__matrix_data_type_allowed(app_config):
raise DatasetAccessError(
f"{self.location} does not have an allowed type: {str(self.matrix_data_type)}")
raise DatasetAccessError(f"{self.location} does not have an allowed type: {str(self.matrix_data_type)}")
if self.matrix_data_type == MatrixDataType.H5AD:
from server.data_anndata.anndata_adaptor import AnndataAdaptor
+9 -7
View File
@@ -1,5 +1,6 @@
import os
import json
import logging
from server.common.utils import dtype_to_schema
from server.common.errors import DatasetAccessError, ConfigurationError
from server.common.utils import path_join
@@ -52,7 +53,8 @@ class CxgAdaptor(DataAdaptor):
def pre_load_validation(data_locator):
location = data_locator.uri_or_path
if not CxgAdaptor.isvalid(location):
raise DatasetAccessError(f"cxg matrix is not valid: {location}")
logging.error(f"cxg matrix is not valid: {location}")
raise DatasetAccessError("cxg matrix is not valid")
@staticmethod
def file_size(data_locator):
@@ -169,12 +171,12 @@ class CxgAdaptor(DataAdaptor):
p = self.get_path(name)
try:
array = tiledb.DenseArray(p, mode="r", ctx=self.tiledb_ctx)
except tiledb.libtiledb.TileDBError as e:
raise AttributeError(str(e))
except tiledb.libtiledb.TileDBError:
raise DatasetAccessError(name)
self.arrays[name] = array
return array
except tiledb.libtiledb.TileDBError as e:
raise AttributeError(str(e))
except tiledb.libtiledb.TileDBError:
raise DatasetAccessError(name)
def get_embedding_array(self, ename, dims=2):
array = self.open_array(f"emb/{ename}")
@@ -210,8 +212,8 @@ class CxgAdaptor(DataAdaptor):
var = self.open_array("obs")
try:
data = var.query(attrs=[term_name])[:][term_name]
except tiledb.libtiledb.TileDBError as e:
raise AttributeError(str(e))
except tiledb.libtiledb.TileDBError:
raise DatasetAccessError("query_obs")
return data
def get_obs_names(self):
+4 -4
View File
@@ -41,7 +41,7 @@ class ViewDataset(TaskSet):
with self.client.get(
f"{self.dataset}{API}/annotations/var?annotation-name={self.var_index_name()}",
headers={"Accept": "application/octet-stream"},
catch_response=True
catch_response=True,
) as r:
if r.status_code == 200:
df = decode_fbs.decode_matrix_FBS(r.content)
@@ -112,7 +112,7 @@ class ViewDataset(TaskSet):
self.client.get(
f"{self.dataset}{API}/annotations/var?annotation-name={self.parent.var_index_name()}",
headers={"Accept": "application/octet-stream"},
stream=True
stream=True,
).close()
group = Group()
@@ -126,7 +126,7 @@ class ViewDataset(TaskSet):
self.client.get(
f"{self.dataset}{API}/annotations/obs?annotation-name={name}",
headers={"Accept": "application/octet-stream"},
stream=True
stream=True,
).close()
obs_names = self.parent.obs_annotation_names()
@@ -150,7 +150,7 @@ class ViewDataset(TaskSet):
f"{self.dataset}{API}/data/var",
data=json.dumps(filter),
headers={"Content-Type": "application/json", "Accept": "application/octet-stream"},
stream=True
stream=True,
).close()