diff --git a/server/app/app.py b/server/app/app.py index 4dfd2a67..1330774e 100644 --- a/server/app/app.py +++ b/server/app/app.py @@ -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 += "Hosted Cellxgene" data += "

Welcome to cellxgene

" - 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 += "
Select one of these datasets...
" - data += "" - except Exception as e: - data += f'
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 += "
Select one of these datasets...
" + data += "" data += "" + 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: diff --git a/server/common/annotations.py b/server/common/annotations.py index 0e0f1bb1..45e53122 100644 --- a/server/common/annotations.py +++ b/server/common/annotations.py @@ -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 diff --git a/server/common/rest.py b/server/common/rest.py index 5f920bb4..111aca2d 100644 --- a/server/common/rest.py +++ b/server/common/rest.py @@ -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) diff --git a/server/compute/scanpy.py b/server/compute/scanpy.py index 679d9f4e..cebafabd 100644 --- a/server/compute/scanpy.py +++ b/server/compute/scanpy.py @@ -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={}): diff --git a/server/data_anndata/anndata_adaptor.py b/server/data_anndata/anndata_adaptor.py index d8441e4f..4442ff13 100644 --- a/server/data_anndata/anndata_adaptor.py +++ b/server/data_anndata/anndata_adaptor.py @@ -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) diff --git a/server/data_common/data_adaptor.py b/server/data_common/data_adaptor.py index 4fd9d7ae..4b22264b 100644 --- a/server/data_common/data_adaptor.py +++ b/server/data_common/data_adaptor.py @@ -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 diff --git a/server/data_common/matrix_loader.py b/server/data_common/matrix_loader.py index a775868a..65145b40 100644 --- a/server/data_common/matrix_loader.py +++ b/server/data_common/matrix_loader.py @@ -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 diff --git a/server/data_cxg/cxg_adaptor.py b/server/data_cxg/cxg_adaptor.py index 3439ea2a..204c8c04 100644 --- a/server/data_cxg/cxg_adaptor.py +++ b/server/data_cxg/cxg_adaptor.py @@ -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): diff --git a/server/locust/locustfile.py b/server/locust/locustfile.py index 9a900491..9e2ee25c 100644 --- a/server/locust/locustfile.py +++ b/server/locust/locustfile.py @@ -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()