Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor error response #85

Merged
merged 11 commits into from
Feb 20, 2019
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
+ `POST /api/v1/metric` has been implemented (#74)
+ `PUT /api/v1/metric/<metric_name>` has been implemented (#75)
+ `PATCH /api/v1/metric/<metric_name>` has been implemented (#83)
+ Error responses for the REST API have been refactored (#85)


## v0.3.0 (2019-01-28)
Expand Down
175 changes: 175 additions & 0 deletions src/trendlines/error_responses.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
# -*- coding: utf-8 -*-
"""
"""
from enum import Enum

from flask import jsonify

from trendlines import logger
from . import utils


class ErrorResponseType(Enum):
NO_DATA = 1
NOT_FOUND = 2
INVALID_REQUEST = 3
ALREADY_EXISTS = 4
INTEGRETY_ERROR = 5

def __str__(self):
return self.name.lower().replace("_", "-")

@property
def url(self):
base = "https://trendlines.readthedocs.io/en/latest/error-responses"
return base + "#" + str(self)


class ErrorResponse(object):

@classmethod
def metric_not_found(cls, name):
detail = "The metric '{}' does not exist".format(name)
return error_response(404, ErrorResponseType.NOT_FOUND, detail)

@classmethod
def metric_has_no_data(cls, name):
detail = "No data exists for metric '{}'.".format(name)
return error_response(404, ErrorResponseType.NO_DATA, detail)

@classmethod
def metric_already_exists(cls, name):
detail = "The metric '{}' already exists.".format(name)
return error_response(409, ErrorResponseType.ALREADY_EXISTS, detail)

@classmethod
def unique_metric_name_required(cls, old, new):
detail = ("Unable to change metric name '{}': target name '{}'"
" already exists.")
detail = detail.format(old, new)
return error_response(409, ErrorResponseType.INTEGRETY_ERROR, detail)

@classmethod
def missing_required_key(cls, key):
detail = "Missing required key 'name'"
return error_response(400, ErrorResponseType.INVALID_REQUEST, detail)


class Rfc7807ErrorResponse(object):
"""
An error response object that (mostly) conforms to `RFC 7807`_.

Parameters
----------
type_ : str
A URI reference that identifies the problem type. Typically is a link
to human-readable documentation for the problem type.
title : str, optional
A short, human-readable summary of the problem type. It SHOULD NOT
change from occurrence to occurence, except for the purposes of
localization.
status : int, optional
The HTTP status code generated by the origin server.
detail : str, optional
A human-readable explaination specific to this occurence of the
problem.
instance : str, optional
A URI reference that identifies the specific occurence of the
problem.
others : dict
Additional key-value pairs to be returned.

_`RCF 7807`: https://tools.ietf.org/html/rfc7807
"""
_content_type = "application/problem+json"

def __init__(self, type_, title=None, status=None, detail=None,
instance=None, **kwargs):
# Required
self.type = type_
# Recommended
self.title = title
self.status = status
self.detail = detail
# Optional
self.instance = instance

self.others = kwargs

@property
def content_type(self):
return Rfc7807ErrorResponse._content_type

def as_dict(self):
"""
Return this object as a Python dictionary.
"""
d = {"type": self.type}
other_keys = ['title', 'status', 'detail', 'instance']
for k in other_keys:
if getattr(self, k) is not None:
d[k] = getattr(self, k)

return {**d, **self.others}

def as_response(self):
"""
Return this object as a flask Response object.

Note that this will only work within a application context.

The only difference between this and ``flask.jsonify`` is that the
mimetype is modified to fit with RFC 7807.
"""
with utils.adjust_jsonify_mimetype(self.content_type):
return jsonify(self.as_dict())


def error_response(http_status, type_, detail):
"""
Create an error response for HTTP/JSON.

This handles some of the minor implementation details of
:class:`error_responses.Rfc7807ErrorResponse` such as passing around the http
status code, logging, and setting the ``title`` value.

Parameters
----------
http_status : int
The HTTP status code to return.
type_ : :class:`error_responses.ErrorResponseType
The general type of error.
detail : str
Specific error details, typically including how to resolve the issue.

Returns
-------
(response, status_code)
A two-tuple suitible for being the return value of a flask route.

Notes
-----
The ``Title`` key for RFC8708 is currently derived from the type. This
may change in the future.

Example Usage
-------------

code-block:: python
@app.route("/foo", methods=["GET"])
def get_thing():
try:
thing = find_thing()
except NotFound:
return error_responses.error_response(
404,
error_responses.ErrorResponseType.NOT_FOUND,
"The thing was not found.",
)
return jsonify(thing), 200

"""
title = type_.name
resp = Rfc7807ErrorResponse(type_.url, title, http_status, detail)
logger.warning("API error %s: %s" % (title, detail))
return resp.as_response(), http_status
126 changes: 12 additions & 114 deletions src/trendlines/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

from trendlines import logger
from . import db
from .error_responses import ErrorResponse
from . import utils

pages = Blueprint('pages', __name__)
Expand Down Expand Up @@ -94,28 +95,10 @@ def get_data_as_json(metric):
raw_data = db.get_data(metric)
units = db.get_units(metric)
except DoesNotExist:
http_status = 404
detail = "The metric '{}' does not exist.".format(metric)
resp = utils.Rfc7807ErrorResponse(
type_="metric-not-found",
title="Metric not found",
status=http_status,
detail=detail,
)
logger.warning("API error: %s" % detail)
return resp.as_response(), http_status
return ErrorResponse.metric_not_found(metric)

if len(raw_data) == 0:
http_status = 404
detail = "No data exists for metric '{}'.".format(metric)
resp = utils.Rfc7807ErrorResponse(
type_="no-data",
title="No data for metric",
status=http_status,
detail=detail,
)
logger.warning("API error: %s" % detail)
return resp.as_response(), http_status
return ErrorResponse.metric_has_no_data(metric)

data = utils.format_data(raw_data, units)

Expand Down Expand Up @@ -191,16 +174,7 @@ def get_metric_as_json(metric):
try:
raw_data = db.Metric.get(db.Metric.name == metric)
except DoesNotExist:
http_status = 404
detail = "The metric '{}' does not exist".format(metric)
resp = utils.Rfc7807ErrorResponse(
type_="metric-not-found",
title="Metric not found",
status=http_status,
detail=detail,
)
logger.warning("API error: %s" % detail)
return resp.as_response(), http_status
return ErrorResponse.metric_not_found(metric)

data = utils.format_metric_api_result(raw_data)

Expand Down Expand Up @@ -235,30 +209,12 @@ def post_metric():
try:
metric = data['name']
except KeyError:
http_status = 400
detail = "Missing required key 'name'."
resp = utils.Rfc7807ErrorResponse(
type_="invalid-request",
title="Missing required JSON key.",
status=http_status,
detail=detail,
)
logger.warning("API error: %s" % detail)
return resp.as_response(), http_status
return ErrorResponse.missing_required_key('name')

try:
exists = db.Metric.get(db.Metric.name == metric) is not None
if exists:
http_status = 409
detail = "The metric '{}' already exists.".format(metric)
resp = utils.Rfc7807ErrorResponse(
type_="already-exists",
title="Metric already exists",
status=http_status,
detail=detail,
)
logger.warning("API error: %s" % detail)
return resp.as_response(), http_status
return ErrorResponse.metric_already_exists(metric)
except DoesNotExist:
logger.debug("Metric does not exist. Able to create.")

Expand Down Expand Up @@ -296,16 +252,7 @@ def delete_metric(metric):
found = db.Metric.get(db.Metric.name == metric)
found.delete_instance()
except DoesNotExist:
http_status = 404
detail = "The metric '{}' does not exist".format(metric)
resp = utils.Rfc7807ErrorResponse(
type_="metric-not-found",
title="Metric not found",
status=http_status,
detail=detail,
)
logger.warning("API error: %s" % detail)
return resp.as_response(), http_status
return ErrorResponse.metric_not_found(metric)
else:
return "", 204

Expand Down Expand Up @@ -358,32 +305,14 @@ def put_metric(metric):
old_lower = metric.lower_limit
old_upper = metric.upper_limit
except DoesNotExist:
http_status = 404
detail = "The metric '{}' does not exist".format(metric)
resp = utils.Rfc7807ErrorResponse(
type_="metric-not-found",
title="Metric not found",
status=http_status,
detail=detail,
)
logger.warning("API error: %s" % detail)
return resp.as_response(), http_status
return ErrorResponse.metric_not_found(metric)

# Parse our json.
# TODO: possible to replace with peewee.dict_to_model?
try:
name = data['name']
except KeyError:
http_status = 400
detail = "Missing required key 'name'."
resp = utils.Rfc7807ErrorResponse(
type_="invalid-request",
title="Missing required JSON key.",
status=http_status,
detail=detail,
)
logger.warning("API error: %s" % detail)
return resp.as_response(), http_status
return ErrorResponse.missing_required_key('name')

# All other fields we assume to be None if they're missing.
units = data.get('units', None)
Expand All @@ -400,18 +329,7 @@ def put_metric(metric):
metric.save()
except IntegrityError:
# Failed the unique constraint on Metric.name
http_status = 409
detail = ("Unable to change metric name '{}': target name '{}'"
" already exists.")
detail = detail.format(old_name, name)
resp = utils.Rfc7807ErrorResponse(
type_="integrity-error",
title="Constraint Failure",
status=http_status,
detail=detail,
)
logger.warning("API error: %s" % detail)
return resp.as_response(), http_status
return ErrorResponse.unique_metric_name_required(old_name, name)

rv = {
"old_value": {
Expand Down Expand Up @@ -477,35 +395,15 @@ def patch_metric(metric):
old_lower = metric.lower_limit
old_upper = metric.upper_limit
except DoesNotExist:
http_status = 404
detail = "The metric '{}' does not exist".format(metric)
resp = utils.Rfc7807ErrorResponse(
type_="metric-not-found",
title="Metric not found",
status=http_status,
detail=detail,
)
logger.warning("API error: %s" % detail)
return resp.as_response(), http_status
return ErrorResponse.metric_not_found(metric)

metric = update_model_from_dict(metric, data)

try:
metric.save()
except IntegrityError:
# Failed the unique constraint on Metric.name
http_status = 409
detail = ("Unable to change metric name '{}': target name '{}'"
" already exists.")
detail = detail.format(old_name, metric.name)
resp = utils.Rfc7807ErrorResponse(
type_="integrity-error",
title="Constraint Failure",
status=http_status,
detail=detail,
)
logger.warning("API error: %s" % detail)
return resp.as_response(), http_status
return ErrorResponse.unique_metric_name_required(old_name, metric.name)

old = {
"name": old_name,
Expand Down