RESTException.get_errors() fails when the object is instantiated with a string

[borellim]

Hello. I was playing with invenio-s3, and I made a simple flask app to test it:

# ... omissis ...
@blueprint.route('/download', methods=('GET',))
@login_required
def download():
    s3fs = S3FSFileStorage('s3://test_bucket')
    return s3fs.send_file('test-file')

The above code is incorrect, because it's missing the object key (it should be S3FSFileStorage('s3://test_bucket/file_name')), but I could not see the correct error message because of what I think is a bug in invenio-rest. Instead I got this in my logs:

Traceback (most recent call last):
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/gunicorn/workers/sync.py", line 135, in handle
    self.handle_request(listener, req, client, addr)
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/gunicorn/workers/sync.py", line 176, in handle_request
    respiter = self.wsgi(environ, resp.start_response)
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/flask/app.py", line 2328, in __call__
    return self.wsgi_app(environ, start_response)
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/werkzeug/middleware/dispatcher.py", line 66, in __call__
    return app(environ, start_response)
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/flask/app.py", line 2314, in wsgi_app
    response = self.handle_exception(e)
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/flask/app.py", line 1760, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/flask/_compat.py", line 36, in reraise
    raise value
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/flask/app.py", line 2311, in wsgi_app
    response = self.full_dispatch_request()
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/flask/app.py", line 1835, in full_dispatch_request
    return self.finalize_request(rv)
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/flask/app.py", line 1850, in finalize_request
    response = self.make_response(rv)
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/flask/app.py", line 1993, in make_response
    rv = self.response_class.force_type(rv, request.environ)
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/werkzeug/wrappers/base_response.py", line 269, in force_type
    response = BaseResponse(*_run_wsgi_app(response, environ))
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/werkzeug/wrappers/base_response.py", line 26, in _run_wsgi_app
    return _run_wsgi_app(*args)
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/werkzeug/test.py", line 1119, in run_wsgi_app
    app_rv = app(environ, start_response)
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/werkzeug/exceptions.py", line 181, in __call__
    response = self.get_response(environ)
  File "/home/ubuntu/.virtualenvs/archive/lib/python3.5/site-packages/werkzeug/exceptions.py", line 172, in get_response
    return Response(self.get_body(environ), self.code, headers)
  File "/home/ubuntu/invenio-rest/invenio_rest/errors.py", line 75, in get_body
    errors = self.get_errors()
  File "/home/ubuntu/invenio-rest/invenio_rest/errors.py", line 62, in get_errors
    return [e.to_dict() for e in self.errors] if self.errors else None
  File "/home/ubuntu/invenio-rest/invenio_rest/errors.py", line 62, in <listcomp>
    return [e.to_dict() for e in self.errors] if self.errors else None
AttributeError: 'str' object has no attribute 'to_dict'

The reason is that RESTException.get_errors() assumes that self.errors is a list, but it can be a string instead. In my case, S3FSFileStorage.send_file() is raising a StorageError (a subclass of RESTException) using a simple string as construction argument, which then is stored in self.errors. There are other places in other modules that do the same (example).

I solved the issue with this change, but I expect it to be suboptimal:

--- a/invenio_rest/errors.py
+++ b/invenio_rest/errors.py
@@ -59,7 +59,12 @@ class RESTException(HTTPException):

         :returns: A list containing a dictionary representing the errors.
         """
-        return [e.to_dict() for e in self.errors] if self.errors else None
+        if not self.errors:
+            return None
+        elif isinstance(self.errors, list) or isinstance(self.errors, tuple):
+            return [e.to_dict() for e in self.errors]
+        else:
+            return self.errors

     def get_description(self, environ=None):
         """Get the description."""

Now I could see my error in the browser and I could fix it.