[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wink-dev
Subject:    [jira] Commented: (WINK-234) improve debug and return code for
From:       "Bryant Luk (JIRA)" <jira () apache ! org>
Date:       2009-11-30 20:14:20
Message-ID: 1927414036.1259612060637.JavaMail.jira () brutus
[Download RAW message or body]


    [ https://issues.apache.org/jira/browse/WINK-234?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12783828#action_12783828 \
] 

Bryant Luk commented on WINK-234:
---------------------------------

I think #1 is probably the best option.

Assuming that no providers are added/removed, a private volatile boolean could be \
used to cache whether or not a provider exists for the null check.

For the providers that we control, adding debug might be a good thing if the logic is \
not simple.  The ProvidersRegistry does have some trace when isReadable/isWritable is \
called (but obviously none inside the actual isWritable/isReadable methods).

For the readFrom methods, I think JAX-RS 1.1. C003 clarifies returning an empty \
object.  For the writeTo methods, explicitly setting a 500 error message may be too \
late (i.e. the HTTP response is already flushed/committed due to a large message \
body) depending on the case.

> improve debug and return code for unfound writer
> ------------------------------------------------
> 
> Key: WINK-234
> URL: https://issues.apache.org/jira/browse/WINK-234
> Project: Wink
> Issue Type: Improvement
> Components: Common, Server
> Affects Versions: 1.1
> Reporter: Mike Rheinheimer
> Fix For: 1.1
> 
> 
> Recently a user got an NPE on the server side when he had inadvertently forgotten \
> to package the jettison provider with the DefectAsset sample.  The NPE: \
> java.lang.NullPointerException  at \
> org.apache.wink.common.internal.providers.entity.atom.AtomEntrySyndEntryProvider.writeTo(AtomEntrySyndEntryProvider.java:96)
>  This was due, of course, to not being able to find a provider to handler \
> DefectAsset to json data transformation.  This problem could have been prevented in \
> one of two ways: 1)  the isWriteable method in AtomEntrySyndEntryProvider could \
> have checked for a writer and returned false when one was not found, or 2)  the \
> writeTo method in AtomEntrySyndEntryProvider could have detected a null return from \
> providers.getMessageBodyWriter, and responded with some debug output and a 500 \
> response code. As it is now, a user would have to inspect source code to determine \
> the possible cause of the NPE.  In poking around at the various providers we have \
> in Wink, I see both strategies employed.  Do we want to establish some consistency \
> on this?  If so, which is the most appropriate?  My understanding is that we want \
> isWriteable to be as fast as possible, so perhaps #2 makes the most sense for \
> performance.  For usability, however, we should never even call writeTo if \
> isWriteable doesn't pass, so perhaps #1 is best. Besides the isWriteable and \
> writeTo problem, I did not see any such detection or debug output in any of the \
> isReadable and readFrom methods in the various providers.  I think we should do the \
> same in these areas?  Any opinions? See the following provider that prompted the \
> original user's question: \
> org.apache.wink.common.internal.providers.entity.atom.AtomEntrySyndEntryProvider \
> See this provider that detects null in the isWriteable method: \
> org.apache.wink.common.internal.providers.entity.app.CategoriesProvider See this \
> provider for an example of outputting debug and explicitly returning 500 http code: \
> org.apache.wink.common.internal.model.AnyContentHandler I'll be glad to go through \
> and adjust the providers once we reach a consensus on this Jira.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[prev in list] [next in list] [prev in thread] [next in thread]