Thread Rating:
  • 0 Votes - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Allow Java devices to customize action errors
29-07-2012, 06:40 PM
Post: #1
Allow Java devices to customize action errors
I've added support in the Java device bindings to customizing the error code and error text for action errors. This is required to make the device comply with UPnP specifications saying which codes should be used for certain errors.

The changes are based on how this is done for the C# bindings, with a couple of small differences described below.

1) Replace ActionError.java by the following code:
Code:
package org.openhome.net.device;

/**
* Thrown when an invoked action fails.  Will not be visible to external code.
*/
public class ActionError extends RuntimeException
{
    private int iErrorCode;

    public ActionError()
    {
        super();
    }

    public ActionError(String aMessage)
    {
        super(aMessage);
    }

    public ActionError(String aMessage, int aErrorCode)
    {
        super(aMessage);
        iErrorCode = aErrorCode;
    }

    /**
     * Return the error code.
     *
     * @return  the error code.
     */
    public int getErrorCode()
    {
        return iErrorCode;
    }
}

2) Add a new method reportActionError() at line 380 in the DvInvocation.java, as follows:
Code:
/**
     * Report an error from action processing in provider code.
     *
     * @param aError       the {@link #ActionError} exception object.
     * @param aActionName  the action name.
     */
    public void reportActionError(ActionError aError, String aActionName)
    {
        String msg = aError.getMessage();
        if (msg == null)
        {
            msg = String.format("Action %s failed", aActionName);
        }
        int errCode = aError.getErrorCode();
        if (errCode == 0)
        {
            errCode = 501;
        }
        reportError(errCode, msg);
    }

3) Change line 339 of DvUpnpJava.tt from:
Code:
invocation.reportError(501, "Invalid XML");
to:
Code:
invocation.reportActionError(ae, "<#=a.name#>");

The differences from the corresponding C# code are as follows:

a) The error code is stored as an integer field in the ActionError class instead of being added as a prefix to the text message.

b) The C# code adds 800 to the error code specified by the provider, and the Java code uses the provider-specified code "as is". Using the provider error code "as is" makes it much more straightforward for the provider to send error codes below 800, such as 710 to report an invalid container ID.

I'd also like to add the ability for Java control points to obtain the error code and text from the ProxyError object, but this requires a core runtime change to get the Error object from the Invocation object's private iError field. Is there some reason why ohNet doesn't expose this information to the language bindings?
Find all posts by this user
30-07-2012, 08:25 AM
Post: #2
RE: Allow Java devices to customize action errors
Most of these changes sound good and I'll be happy to apply them. I don't want to introduce differences in behaviour between language bindings though; this would make overview documentation far harder. So if you want to be able to set error codes outside the 800s, you'll need to convince me that the C#/C++ APIs are deficient and should be fixed :-).

I thought the device architecture doc only allowed for errors between 800-899. Where does 710 - Invalid Container ID come from? Is this another case where the media server device description implies changes to the spec without going to the trouble of actually updating the spec itself?

Regarding control point retrieval of error details, I agree that this is desirable but don't think it'll be as simple as exposing Invocation::iError. Wouldn't you want the info from the FaultCode xml in the action response? To guard against me implementing the wrong thing, can you describe how you'll use this info please? A complete enumeration of the error cases you want to handle (or expose to users) would be very useful.
Find all posts by this user
30-07-2012, 10:35 AM
Post: #3
RE: Allow Java devices to customize action errors
(30-07-2012 08:25 AM)simonc Wrote:  Most of these changes sound good and I'll be happy to apply them. I don't want to introduce differences in behaviour between language bindings though; this would make overview documentation far harder. So if you want to be able to set error codes outside the 800s, you'll need to convince me that the C#/C++ APIs are deficient and should be fixed :-).

I thought the device architecture doc only allowed for errors between 800-899. Where does 710 - Invalid Container ID come from? Is this another case where the media server device description implies changes to the spec without going to the trouble of actually updating the spec itself?

The UPnP Device Architecture 1.1 and the ContentDirectory:1 Service Template 1.01 are consistent on what errors can be returned by a device.

In Table 3.3 on page 82 of the UPnP Device Architecture 1.1 spec, the listed errors 600 through 605 can be raised by provider code. It should be possible for ohNet to take care of raising 602 errors, but it doesn't seem to do this at the moment.

Table 3.3 also reserves codes in the range 613-699 and 700-799 for definition by other UPnP specs. In Table 11 on page 35 of the ContentDirectory:1 spec, codes 701 through 720 are defined, and a ContentDirectory:1 implementation needs to be able to produce these error codes.

In both table 3.3 and table 11, codes 800-899 are reserved for use by UPnP vendors. So if a vendor is defining a proprietary service and needs some error codes for the actions of that service, these should go in the 800-899 range.

Quote:Regarding control point retrieval of error details, I agree that this is desirable but don't think it'll be as simple as exposing Invocation::iError. Wouldn't you want the info from the FaultCode xml in the action response? To guard against me implementing the wrong thing, can you describe how you'll use this info please? A complete enumeration of the error cases you want to handle (or expose to users) would be very useful.

The information that I'd like to see is the error code and error description. This information is present in the Error object and can be retrieved by the Code() and Description() methods. So if the Java or C# bindings have some way of getting the Error object, they should be able to add this information to the ProxyError object that's thrown to user code.

These errors might be standard errors defined by the UPnP specs or they might be properietary errors defined by a proprietary service. At a minimum, I think the control point would want to log the error code and description to aid problem determination. For some errors such as 603 and 605, it may be possible for the control point to modify and/or retry the request. For error 604, the control point should report this through its UI. For error 602, the control point may be able to work around the lack of device support for an optional action by invoking some other action or limiting the functionality that it provides to the user.

For the ContentDirectory:1 service, errors 701 and 710 might indicate that the control point's cache of server container data is out of date (for example, following a server rescan) and needs to be refreshed from the root container.

For proprietary services that use errors in the 800-899 range, there are likely to be similar cases where some errors can be recovered by the control point.
Find all posts by this user
30-07-2012, 11:08 AM
Post: #4
RE: Allow Java devices to customize action errors
Thanks, you're right that the C++ and C# APIs are wrong. I'll change them in line with your proposals.

The CP changes maybe won't be quite as straightforward as you suggested (the int in Invocation::iError is an enum, not an http status code) but something along this line seems very sensible. I'll change this at the same time as the above.

I'll probably not have time to look at this until next week. Will that cause you any problems?
Find all posts by this user
30-07-2012, 12:11 PM
Post: #5
RE: Allow Java devices to customize action errors
(30-07-2012 11:08 AM)simonc Wrote:  The CP changes maybe won't be quite as straightforward as you suggested (the int in Invocation::iError is an enum, not an http status code) but something along this line seems very sensible. I'll change this at the same time as the above.

Yes, I see that now.

Quote:I'll probably not have time to look at this until next week. Will that cause you any problems?

The server-side code in the Java bindings is more urgent for me than the client-side support (and probably simpler as well). Would it be possible to do that part first?
Find all posts by this user
08-08-2012, 01:07 PM
Post: #6
RE: Allow Java devices to customize action errors
Thanks for applying these changes.

Unforunately they haven't quite been applied correctly. The change to DvUpnpJava.tt has ben applied as
Code:
invocation.reportActionError(ae, "Invalid XML");
instead of
Code:
invocation.reportActionError(ae, "<#=a.name#>");

The second argument to reportActionError() should be the name of the action. It's used by reportActionError() to produce a default message "Action xxxx failed".

If you look at the equivalent code in the C# bindings, you'll see that it does the same thing.
Find all posts by this user
08-08-2012, 01:09 PM
Post: #7
RE: Allow Java devices to customize action errors
(08-08-2012 01:07 PM)simoncn Wrote:  Thanks for applying these changes.

Unforunately they haven't quite been applied correctly. The change to DvUpnpJava.tt has ben applied as
Code:
invocation.reportActionError(ae, "Invalid XML");
instead of
Code:
invocation.reportActionError(ae, "<#=a.name#>");

The second argument to reportActionError() should be the name of the action. It's used by reportActionError() to produce a default message "Action xxxx failed".

If you look at the equivalent code in the C# bindings, you'll see that it does the same thing.

Oops, well spotted. On closer inspection, the C# code is wrong too. I'll fix both later today.
Find all posts by this user
08-08-2012, 03:45 PM (This post was last modified: 08-08-2012 03:46 PM by simonc.)
Post: #8
RE: Allow Java devices to customize action errors
This evening's updates will include an error code in ProxyError. This code corresponds to the fault's errorCode element in a UPnP action response. Which itself may have come from an ActionError instance.

Native code also makes the errorDescription element available. It'd be possible to add this to ProxyError; diffs for this would be very welcome :-)
Find all posts by this user
08-08-2012, 11:20 PM
Post: #9
RE: Allow Java devices to customize action errors
(08-08-2012 03:45 PM)simonc Wrote:  Native code also makes the errorDescription element available. It'd be possible to add this to ProxyError; diffs for this would be very welcome :-)

I've got this working. I should be able to produce a set of diffs by Friday. Smile
Find all posts by this user
11-08-2012, 12:26 PM
Post: #10
RE: Allow Java devices to customize action errors
I'm attaching a zip file containing diffs for the following:

OpenHome/Net/Bindings/Java/Invocation.c
OpenHome/Net/Bindings/Java/Invocation.h
OpenHome/Net/Bindings/Java/org/openhome/net/controlpoint/Invocation.java
OpenHome/Net/Bindings/Java/org/openhome/net/controlpoint/SyncProxyAction.java
OpenHome/Net/Bindings/Java/org/openhome/net/controlpoint/ProxyError.java (complete replacement)
OpenHome/Net/T4/Templates/CpJavaUpnp.tt

I'm doing this instead of putting code inline here because of double-spacing problems that might have been caused when inline code was copied and pasted. I've noticed this problem in ActionError.java, ProxyError.java, and lines 380 to 415 of DvInvocation.java.

While testing these changes, I found some problems with error handling in SyncProxyAction.java and Invocation.java. These diffs include fixes for these problems.


Attached File(s)
.zip  diff1.zip (Size: 2.5 KB / Downloads: 3)
Find all posts by this user


Forum Jump: