What I was proposing was the addition of a method which
would indicate failure before starting. This method
would only be called before OnStartRequest... it could
not be called in place of OnStopRequest. Doing so would
break the rule that OnStartRequest/OnStopRequest must
come as a pair.
I proposed calling it OnFailRequest, in an effort to
annotate the condition of failure before starting to
process a request. Once started, a request can only
error-out via OnStopReqest and not via OnFailRequest.
Perhaps OnFailRequest should instead be named
OnFailedToStartRequest() ??
OnFailRequest is very different from OnStopRequest. A
listener would only see OnFailRequest if the request could
not be started.
There are two alternatives to an OnFailRequest method.
The first, is to replace it with OnStartRequest followed
directly by OnStopRequest (with OnStopRequest conveying
the error condition). This is what the protocol/transport
implementations currently do.
The second approach would be to add an nsresult parameter
to OnStartRequest. This parameter would have to be checked
by the listener... if indicating failure, then the listener
would know not to expect OnDataAvailable events. But, the
protocol/transport implementations would probably still
want to send an OnStopRequest... just to preserve the
pairing of OnStartRequest with OnStopRequest.
The second approach seems extraneous. The first approach
works just fine, except that it can involve extra overhead
on the part of the listener... that is, they may try to
do setup for OnDataAvailable even though there aren't going
to be any OnDataAvailable events.
NOTE: the point of OnStartRequest is to tell the listener
to prepare for OnDataAvailable. That means, that the
listener should be doing all initialization in OnStartRequest
and NOT on the first invocation of OnDataAvailable. There
certainly isn't anything requiring the listeners to follow
this convention, but it just seems like the right thing
to do from the standpoint of code clarity.
The second approach, much like OnFailRequest, is an API
change. IMO if we're going to make an API change to improve
on the current situation, then it might as well be the
addition of OnFailRequest. This just seems a lot cleaner
to me.
Perhaps the underlying issue is not that important.. perhaps
it is okay to call OnStartRequest even though the caller
knows it will not be calling OnDataAvailable. Perhaps the
overhead of calling OnStartRequest is insignificant. Perhaps
this issue is best left as something to revisit in the future..
Darin
Judson Valeski wrote:
> Darin has proposed a new method: nsIStreamObserver::OnFailure() (or
> something like that).
>
> Currently nsIStreamObserver has OnStartRequest and OnStopRequest. Those
> callbacks indicate when the "transaction" has started and when it has
> stopped.
>
> Sigs for each (grrr. I hate it when sigs change and comments don't
> reflect it!):
>
> /** * Called to signify the beginning of an asyncronous request */
> void onStartRequest(in </seamonkey/ident?i=in> nsIChannel
> </seamonkey/ident?i=nsIChannel> channel,
> in </seamonkey/ident?i=in> nsISupports </seamonkey/ident?i=nsISupports>
> ctxt);
>
> /**
> * Called to signify the end of an asyncronous request.
> * @param notif - a notification object containing any error code and
> error parameters
> * (may be null if the notification status is NS_OK)
> */
> void onStopRequest(in </seamonkey/ident?i=in> nsIChannel
> </seamonkey/ident?i=nsIChannel> channel,
> in </seamonkey/ident?i=in> nsISupports </seamonkey/ident?i=nsISupports>
> ctxt,
> in </seamonkey/ident?i=in> ! nsresult </seamonkey/ident?i=nsresult>
> status </seamonkey/ident?i=status>,
> in </seamonkey/ident?i=in> wstring statusArg);
>
> Currently, let's say I want to popup a status dialog when for a
> transaction. I would do this when my onStart (ie. throw dialog w/ text
> "Starting Request") was called, then I'd tear it down when my onStop
> (ie. change text to "Error occured", then tear down the dialog) was called.
>
> Under the current semantics (which dictate OnStart|OnStop combinations
> for *every* transaction; whether the transaction is successful or an
> error), I don't have enough info to *not* throw the dialog if there's an
> error. I've committed *something*... some level of setup. In my example
> it was a dialog being thrown and maybe some internal state. In the
> context of a browser, that may be ok??? We always want to indicate that
> the user has started something; I think. (ie. "Connecting to host..." ->
> "Connected ...." -> "Document:done").
>
> You could argue, and some nsIStreamObserver impls support this argument,
> that OnStart is useless. If I don't want to commit anything because
> there might be an error (and I don't want to indicated that something
> has "started"), I'll just have my OnStart() do nothing but return NS_OK.
> If the transaction is successful my nsIStreamLIstener::OnDataAvailable()
> will be called (w/ a status code (maybe an error occured *after the
> connection*)). If the transaction has an error, I can get that info from
> the OnStop()'s error code. So, I can do everything I want using
> nsIStreamListener::OnData() and nsISTreamObserver::OnStop(). That's
> fine, I can do whatever I want w/ my impl. Maybe I don't need OnStart,
> but other's might.
>
> So, we could change the nsIStreamObserver semantics to:
> - you will *always* get an OnStop, indicating the transaction is over
> (check the status param for success or failure).
> - you will only get an OnStart if things are going to work.
>
> Problem w/ the above is that things can fail *after* the transaction has
> started (server timeout for example), so the 2nd behavior cannot be
> achieved (thus it's existance to begin w/).
>
> So, back to Darin's suggestion. We currently provide transaction
> start/stop apis and dictate that errors are indicated in the status
> param of OnStop(). This binds successful transaction assumptions (setup
> costs in OnStart) to the api, when there's certainly the possibility for
> error (in which case you don't want to set anything up). If we provide
> an OnFailure(), it seems we'd be doing double duty in some cases
> (failure cases). If things are looking up, I'll fire an OnStart()
> callback, then the server could timeout, which would cause an OnStop()
> to get fired w/ a failure status code. Would OnFailure get called before
> or after the onstop? Seems redundant. Would the semantics of OnFaiure()
> subsume an OnStop()? then we've just recreated OnStop.
>
> I see the benifit of getting a single callback indicating error. That
> would allow me to not do anything if I wasn't going to have a
> successfull transaction, but it feels like an edge case. I'm thinking we
> should add a nsresult param to OnStart() so the consumer can skip
> initialization/setup if things have already hit the fan at that point.
>
> Jud
