Issue161 add exception conversion by boillodmanuel · Pull Request #969 · restlet/restlet-framework-java

boillodmanuel · 2014-10-26T20:01:22Z

Work

Done:

Add exception conversion in ConverterService
Add content negotiation in StatusFilter
Replace @Status value type from String to int
Remove code in ServerResource

Not done:

Retrieve response entity in ClientResource after an error occurs in the ServerResource

Use cases explained

Simple request returning no entity (no content)

Not changed

curl -x PUT -H 'Accept: text/html’ ‘http://server.com/api/test'

Response headers:

HTTP/1.1 204 No Content

Response: N/A

Exception not annotated with @Status and with Content-Type "text/html"

Not changed

curl -x PUT -H 'Accept: text/html’ ‘http://server.com/api/exception'

Response headers:

HTTP/1.1 500 Internal Server Error
Content-type: text/html; charset=UTF-8
Content-length: 486

Response:

<html>
<head>
   <title>Status page</title>
</head>
<body style="font-family: sans-serif;">
<p style="font-size: 1.2em;font-weight: bold;margin: 1em 0px;">Internal Server Error</p>
<p>The server encountered an unexpected condition which prevented it from fulfilling the request</p>
<p>You can get technical details <a href="http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.5.1">here</a>.<br>
Please continue your visit at our <a href="/">home page</a>.
</p>
</body>
</html>

Exception not annotated with @Status and with Content-Type "application/json" or without Content-Type

New: Content Negotiation

curl -x PUT -H 'Accept: application/json’ ‘http://server.com/api/exception'

Response headers:

HTTP/1.1 500 Internal Server Error
Content-type: application/json

Response:

{
    "code": 500,
    "description": "The server encountered an unexpected condition which prevented it from fulfilling the request",
    "reasonPhrase": "Internal Server Error"
}

Exception annotated with @Status(400) and with Content-Type "text/html"

New: Status code

curl -x PUT -H 'Accept: text/html’ ‘http://server.com/api/notfound'

Response headers:

HTTP/1.1 404 Not Found
Content-type: text/html; charset=UTF-8
Content-length: 439

Response:

<html>
<head>
   <title>Status page</title>
</head>
<body style="font-family: sans-serif;">
<p style="font-size: 1.2em;font-weight: bold;margin: 1em 0px;">Not Found</p>
<p>The server has not found anything matching the request URI</p>
<p>You can get technical details <a href="http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.5">here</a>.<br>
Please continue your visit at our <a href="/">home page</a>.
</p>
</body>
</html>

Exception annotated with @Status(400) and with Content-Type "application/json" or without Content-Type

New: Status Code and Content Negotiation

curl -x PUT -H 'Accept: application/json’ ‘http://server.com/api/notfound'

Response headers:

HTTP/1.1 404 Not Found
Content-type: application/json

Response:

{
     "code":404,
     "reasonPhrase":"Not Found",
     "description":"The server has not found anything matching the request URI"
}

Exception annotated with @Status(value=400, serializeable=true) and with Content-Type "application/json" or without Content-Type

New: Status Code, Exception properties serialization and Content Negotiation

curl -x PUT -H 'Accept: application/json’ ‘http://server.com/api/badparameter'

Response headers:

HTTP/1.1 404 Not Found
Content-type: application/json

Response:

{
    "message": "Parameter age is not of the expected type: number. Value: male",
    "name": "age",
    "type": "number",
    "value": "male"
}

Exception class:

@Status(value = 400, serializeProperties = true)
public class BadParameterTypeException extends RuntimeException {

    private final String name;
    private final String value;
    private final String expectedType;

    public BadParameterTypeException(String name, String value, String expectedType) {
        super("Parameter " + name + " is not of the expected type: " + expectedType + ". Value: " + value);
        this.name = name;
        this.value = value;
        this.expectedType = expectedType;
    }

    @Override
    public String getMessage() {
        return super.getMessage();
    }

    public String getName() {
        return name;
    }

    public String getValue() {
        return value;
    }

    public String getExpectedType() {
        return expectedType;
    }
}

Add content negotiation in StatusFilter

WARNING: StatusService has been removed from GWT Edition

boillodmanuel · 2014-10-27T13:10:51Z

WARNING: StatusService has been removed from GWT Edition. Need to be validated.

boillodmanuel · 2014-10-27T13:49:05Z

Build failed because build of master failed since https://travis-ci.org/restlet/restlet-framework-java/builds/38889517

jlouvel · 2014-10-29T05:56:53Z

Manuel, thanks it looks great overall!

Some comments:

can set the default value of "serializeProperties" to 'true' instead of 'false' to favor automatic conversion? I had in mind that only the presence of @Status would be necessary to trigger this behavior
code style: qualified names are used instead of imports (ex: StatusService)
why do we use a custom bean serializer (based on java.beans) instead of relying on the ConverterService directly? This would allow us to support JSON, YAML and potentially other media types, to use Jackson annotations and so on.
I don't understand the purpose of StatusRepresentation. We already have org.restlet.data.Status
why do we pass converterService, connegService & metadataService as parameters to toRepresentation(...) instead of member variables of StatusService?
why did we remove StatusService from GWT? Isn't it used at all by the code?
do we support deserialization of status back into proper client-side exception?
could we add a setting in StatusService for "debugMode:boolean". If set to false, we should first set those properties to null before invoking the ConverterService. Default setting should be "true"

jlouvel · 2014-10-29T21:41:29Z

I've added a testException() method to JacksonTestCase in order to play with exception serialization and deserialization. I've also changed the default Jackson serialization setting in JacksonRepresentation to not add "null" and empty collections to the output, producing more compact representations.

Other comments:

we should rename "serializeProperties" into just "serialize" or "serializable" to control whether or not we autoconvert the exception into an error representation, or if we just change the HTTP error status.

thboileau · 2014-10-30T10:38:38Z

modules/org.restlet/src/org/restlet/service/StatusService.java

Why are you instantiating a StatusRepresentation?

boillodmanuel · 2014-10-30T17:50:51Z

PR pushed as branch issue161-add-exception-conversion for additional work.

jlouvel · 2014-10-31T06:29:21Z

Just pushed those refactorings/changes:

Added connegService, metadataService and converterService properties to StatusService and pass them via the constructor instead of the toRepresentation and toStatus methods.
Renamed @Status#serializeProperties (default to false) into serialize (default to true).
Deprecated Component#statusService in favor of the Application#statusService property.

Manuel Boillod added 6 commits October 26, 2014 20:30

Add exception conversion to ConverterService- Issue restlet#161

e62bbc9

Add content negotiation in StatusFilter

Remove code which is already done in StatusFilter.java

4368bad

Extract BeanInfoutils from ThrowableSerializer

fb50102

Use StatusRepresentation instead of a map (better for documentation)

507319b

Verify response entity in tests

5eb5379

Fix GWT Edition

dfb6e76

WARNING: StatusService has been removed from GWT Edition

thboileau reviewed Oct 30, 2014
View reviewed changes

modules/org.restlet/src/org/restlet/service/StatusService.java

Copy link
Copy Markdown

Contributor

thboileau Oct 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you instantiating a StatusRepresentation?

boillodmanuel closed this Oct 30, 2014

boillodmanuel mentioned this pull request Oct 30, 2014

Issue161 add exception conversion #974

Closed

boillodmanuel assigned jlouvel Oct 30, 2014

boillodmanuel deleted the issue161-add-exception-conversion branch October 30, 2014 17:58

jlouvel assigned boillodmanuel and unassigned jlouvel Oct 31, 2014

jlouvel added this to the 2.3 M4 milestone Oct 31, 2014

jlouvel added the Module: Restlet API label Oct 31, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue161 add exception conversion#969

Issue161 add exception conversion#969
boillodmanuel wants to merge 6 commits intorestlet:masterfrom
boillodmanuel:issue161-add-exception-conversion

boillodmanuel commented Oct 26, 2014

Uh oh!

boillodmanuel commented Oct 27, 2014

Uh oh!

boillodmanuel commented Oct 27, 2014

Uh oh!

jlouvel commented Oct 29, 2014

Uh oh!

jlouvel commented Oct 29, 2014

Uh oh!

thboileau Oct 30, 2014

Uh oh!

boillodmanuel commented Oct 30, 2014

Uh oh!

jlouvel commented Oct 31, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

boillodmanuel commented Oct 26, 2014

Uh oh!

boillodmanuel commented Oct 27, 2014

Uh oh!

boillodmanuel commented Oct 27, 2014

Uh oh!

jlouvel commented Oct 29, 2014

Uh oh!

jlouvel commented Oct 29, 2014

Uh oh!

thboileau Oct 30, 2014

Choose a reason for hiding this comment

Uh oh!

boillodmanuel commented Oct 30, 2014

Uh oh!

jlouvel commented Oct 31, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants