Returns "message" as an object instead of a string, when a BadRequest occurs #179
Conversation
|
@wesleybl thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment: To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
|
@jenkins-plone-org please run jobs |
|
It will be necessary to fix two tests in |
src/plone/rest/errors.py
Outdated
| @@ -57,7 +58,10 @@ def __call__(self): | |||
|
|
|||
| def render_exception(self, exception): | |||
| name = type(exception).__name__ | |||
| message = str(exception) | |||
| if isinstance(exception, BadRequest): | |||
| message = exception.args[0] | |||
There was a problem hiding this comment.
message it looked like this in the restapi test:
[{'message': 'Required input is missing.', 'field': 'email_charset', 'error': RequiredMissing('email_charset')}]Later, we tried to do json.dumps on it:
plone.rest/src/plone/rest/errors.py
Line 43 in 8fdc78c
As we have:
'error': RequiredMissing('email_charset')an error occurs:
TypeError: Object of type RequiredMissing is not JSON serializable
We then need to make all the keys a string.
There was a problem hiding this comment.
I fixed this. Now the message is serializable.
wesleybl
force-pushed
the
error_message
branch
from
e29ee01 to
71391f2
Compare
occurs This makes it possible for the error message to reach the client as an object, avoiding the need for string-to-object conversions.
|
@jenkins-plone-org please run jobs |
If the exception is a BadRequest and the message is a list, return the message with list. If the list elements are dictionaries, convert non-serializable keys and values to string. If it is not a BadRequest, returns the exception as a string.
After: plone/plone.rest#179 BadRequest errors are returned as an object and not as a string. This fix here takes this into account. But for now the tests are skipped, so that the merge can be done before the plone.rest change is in a Plone version.
|
@tisto @jensens @davisagli I fix the Ready for review now. |
|
While I was testing this PR with Volto, I discovered that Volto has a function that can transform the error string into a list: So I'll try to fix plone/volto#1868 without this PR here for now. |
|
It was possible to fix the issue without this PR. See: plone/volto#5808 |
This makes it possible for the error message to reach the client as an object, avoiding the need for string-to-object conversions.
plone/plone.restapi#1755 needs to be merge first.
Fixes #176