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

Exception in Core::Response when handling response from failed JSON deserialize attempt #794

Closed
dboehmer opened this issue Dec 1, 2014 · 6 comments

Comments

@dboehmer
Copy link

dboehmer commented Dec 1, 2014

I'm writing an app handling API requests and need to handle invalid JSON/YAML input with a custom error message. Unfortunately Dancer2 behaves strangely when confronted with invalid JSON data.

I built the following example which should have a server side error and send a valid HTTP error back. Instead the code fails at the client side, too, when accessing the response's content.

use Modern::Perl;
use Plack::Test;
use HTTP::Request::Common;

{
    package App;

    use Dancer2;

    set logger => 'Console';
    set serializer => 'JSON';

    any '/' => sub {
        return request->data;
    };
}

my $app = App->to_app;

test_psgi $app, sub {
    my $cb = shift;

    say $cb->(POST '/', Content => '{"foo":42}')->decoded_content;
    say $cb->(POST '/', Content => 'invalid')->decoded_content;
}

Output from my terminal:

{"foo":42}
[App:852] core @2014-12-01 13:00:47> Failed to deserialize the request: malformed JSON string, neither tag, array, object, number, string or atom, at character offset 0 (before "invalid") at /usr/lib/perl5/site_perl/5.14/JSON.pm line 171. in (eval 166) l. 2
Internal Server Error

Modification of a read-only value attempted at /home/USERNAME/workspace/Dancer2/lib//Dancer2/Core/Response.pm line 72.

How to correctly handle invalid data (after all with Serializer::Mutable) on the server side? What's wrong here that decoded_content() fails?

@xsawyerx xsawyerx added this to the 0.156000 milestone Dec 1, 2014
@dboehmer dboehmer changed the title Expection in Core::Response when handling response from failed JSON deserialize attempt Exception in Core::Response when handling response from failed JSON deserialize attempt Dec 1, 2014
@DavsX
Copy link

DavsX commented Dec 1, 2014

I suggest adding engine.serializer.error hook. Right now in Dancer2::Core::Role::Serializer->(de)serialize the errors we catch at (de)serialization are only logged; but there is no way of processing them.

I believe this is the result of a change, that allowed serializing non-ref values as well.

One option that came to mind is maybe to replace this line; we could save the return value of $self->serialize and after that we could check if it is defined. If it is undef, we could fire an error, possibly calling a hook like 'route.serialization.error'.

@xsawyerx
Copy link
Member

xsawyerx commented Dec 6, 2014

This problem has been described in 734c242.

I'm hoping we would sort this out by throwing a proper error. I'm going to postpone this to the next release, because I want this to coincide with the exception classes issue (#765).

@xsawyerx
Copy link
Member

xsawyerx commented Dec 6, 2014

I've committed a fix for not crashing on modifying a read-only value. It still doesn't produce a 500, which is should. That is a separate topic.

Thank you for opening the issue and supplying the testing code!

@xsawyerx xsawyerx closed this as completed Dec 6, 2014
@dboehmer
Copy link
Author

dboehmer commented Dec 6, 2014

Do you mean presetting the HTTP status code to 500 or returning a 500 response right away? In my current project I need to check the success of the deserialization and return a custom error message if input data was invalid. Please don't make it return automatically.

If I can check body length and response->data that allows me checking for errors. No hook is required for me now (although ideal solution).

@xsawyerx
Copy link
Member

xsawyerx commented Dec 7, 2014

If the serialization crashes, it must return a 500. The fact we don't do it now is a bug, and that needs to be fixed.

You should be able to handle wrong input using the serializer.before hook. I'd be more than happy to add a test to make sure you can handle your special case.

veryrusty added a commit that referenced this issue Dec 6, 2017
Removing HTTP::Body caused the issue reported in #794 to reoccur.
(As HTTP::Body would process JSON content and throw an error).

As Role::Serializer wraps the deserialization in an eval and always
returns (undef on failure), to catch an error occuring; this locally
replaces the serializer log_cb so we get a signal of the failure and
die if necessary.

Its, umm, well, messy and should be considered a workaround (at best).
veryrusty added a commit that referenced this issue Mar 15, 2018
Removing HTTP::Body caused the issue reported in #794 to reoccur.
(As HTTP::Body would process JSON content and throw an error).

As Role::Serializer wraps the deserialization in an eval and always
returns (undef on failure), to catch an error occuring; this locally
replaces the serializer log_cb so we get a signal of the failure and
die if necessary.

Its, umm, well, messy and should be considered a workaround (at best).
@seav
Copy link

seav commented Nov 6, 2018

You should be able to handle wrong input using the serializer.before hook.

This is currently not possible. Looking at the Dancer2::Core::Role::Serializer code, the engine.serializer.before hook is only executed on serialization and not on deserialization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants