0b01001001/spectree

[BUG] Spectree flask plugin is logging request validation errors twice

issamemari opened this issue · 2 comments

Describe the bug
Spectree flask plugin is logging validation errors twice: once in default_before_handler and once in default_after_handler.

This can be seen on this line in the implementation of flask_plugin.py

before(request, response, req_validation_error, None)
if req_validation_error:
    after(request, response, req_validation_error, None)
    assert response  # make mypy happy
    abort(response)

result = func(*args, **kwargs)

The after function is being called if a request validation error has been found. This seems strange. I would imagine the after function should only be called after the wrapped function is called, hence the name after.

This is causing request validation errors to be logged twice, and in the after function they're being logged as 500 errors, which doesn't make sense.

To Reproduce
Steps to reproduce the behavior:

  • Decorate a Flask route with spectree.validate and send an invalid request

Expected behavior
I expect the validation error to be logged only once, with code 422.

Error Message

ERROR:None.spectree.utils:d22f73cdab554422988d90bc909566e6 422 Request Validation Error
ERROR:None.spectree.utils:d22f73cdab554422988d90bc909566e6 500 Response Validation Error

Desktop (please complete the following information):

  • OS: macOS
  • Version 12.2.1

Python Information (please complete the following information):

  • Python=3.8.9
  • spectree=0.12.0
  • flask=1.1.4

Thanks for your feedback.

This is introduced in #70.

The purpose of the after handler is not just for logging. It's kind of the cleanup. Check the discussion at #229.

Will fix this.

After rechecking the logic, I think we should rm the after handler in line 196

before(request, response, req_validation_error, None)
if req_validation_error:
after(request, response, req_validation_error, None)
assert response # make mypy happy
abort(response)

Hi @natasha-aleksandrova, I think you should use before handler here. BTW, the response body is also a JSON when aborting. But I'm not sure if you have some different JSON schemas.