Re: NS_FAILED, NS_SUCCESS, and nsresult return values: suggested guidelines?

Nathan Kurz wrote:
> I'm new to Mozilla and rapidly learning about XPCOM.  I'm trying to
> figure out what the suggested guidelines are for the use of nsresult
> return values.  The particular case that confuses me is what to do
> when the result is not an XPCOM error (NS_ERROR_NOT_IMPLEMENTED), not
> a application error (NS_ERROR_OUT_OF_MEMORY), but an expected case
> that is a configured policy.
> 
> I'm working on some changes to the PAC proxy code to allow it to say
> that it to deny a given URL, basically allowing it to proxy it to a
> black hole without the ensuing problems of actually sending a request
> to the black hole.  I'm not sure how this should signal to a caller
> that 1) the request should not be completed, but 2) this is by design
> and shouldn't be treated as an application error.  Ignoring the
> question of whether a proxy should be really be doing this, am I
> allowed to do this by return value, or should this really use some
> other means?
> 
> The content-policy code does very similar things, but I'm not sure if
> it sets a good example. Take for example this code in
> nsImageFrame.cpp::RealLoadImage():
>   1979   if (aCheckContentPolicy)
>   1980     if (!CanLoadImage(realURI)) return NS_ERROR_FAILURE;
> Is this a good pattern to follow?  Or is this better as
> NS_ERROR_CONTENT_POLICY?  Or (with changes to caller)
> NS_SUCCESS_POLICY_BLOCKED?  Or does proper procedure demand that one
> return NS_OK and somehow signal the blockage by other means?
> 
> Or taking it from the callers point of view, is it good practice to
> analyze the result value one receives by any means other than
> NS_FAILED() or NS_SUCCESS()?
>    if (NS_FAILED(rv) && rv == NS_ERROR_TEMPORARY) 
>        try call again
>    else
>        return rv
> Is this acceptable, or considered bad form: "you takes your lumps"? 
> What about checking for a particular flavor of NS_SUCCESS? Is this
> more or less reasonable?
> 
> On a related note, what should one do with an NS_FAILED(rv) that one
> is not going to try to handle.  Like this code in
> imgLoader::LoadImage():
>   448     rv = NewImageChannel(...)
>   454     if (NS_FAILED(rv))
>   455       return NS_ERROR_FAILURE;
> I'd like to propagate NS_ERROR_PROXY_DENIED through this layer, but it
> obviously doesn't want me to.  Is changing the rv to a generic error
> considered a good way to protect ones caller from a what is
> potentially a module specific error code, or is it meddling in others
> affairs?  In general, should there be an expectation that NS_FAILED
> rv's will be propagated upwards until they are handled?
> 
> I would appreciate guidance.  The code itself seems to vary greatly in
> the choices it makes, and I haven't found any sort of XPCOM style
> guide that answers these questions.  Thanks for any pointers you can
> provide me.

Check out nsError.h.  Like MS-COM, it defines that some result codes are 
errors, and some are not.  All codes with the MSB set are considered 
errors.  From that .h:

#define NS_FAILED(_nsresult) ((_nsresult) & 0x80000000)
#define NS_SUCCEEDED(_nsresult) (!((_nsresult) & 0x80000000))

Thus, it is important that you use NS_FAILED() and NS_SUCCEEDED() rather 
than explicitly checking for non-zero.  NS_OK is but one "success" value.

On the other hand, Python will currently lose this nsresult value - the 
bindings throw the value away if it is not an error code.

Mark.