[Development] async operation error guidance

[André Somers]

Frank Osterfeld schreef op 5-11-2013 17:41:
> On Nov 5, 2013, at 5:12 PM, Tony Van Eerd wrote:
>
>> Most of BlackBerry's APIs are async.
>> We are looking at moving some of these APIs into Qt.
>> Just as one  example, WiFi Direct.
>>
>> I have been attempting to come up with some solid guidelines of how to deal with errors on async operations.
>>
>> Some things to consider:
>>
>> - the initial request may fail. ie
>>
>> 	WifiDirect::connectToDevice(someparams);
>>
>> this could fail immediately if wifi is not on or various other reasons.  So does it return an error:
>>
>> 	ConnectError connectToDevice(SomeParams someparams);
>>
>> or should even the immediate error be made async?
> I'd prefer to have all errors be async, for what's it's worth. That the connect can fail immediately is an implementation detail to me, and
> requires yet another code path for error handling.
I agree. See also http://delta.affinix.com/2006/04/14/delayed-signals/ 
for an explanation. I also dislike the idea to have two different 
classes of errors for the same operation (you even mention two different 
enums in your PDF).

I understand the argument you make in the PDF that you'd like to have 
the errors communicated in a similar way for both synchronous and 
asynchronous APIs. But because there is no way to communicate all 
results, including errors, in this way, I think the preferred way is to 
have a _single_ way of dealing with errors. Even if you immediately 
communicate some errors back, you still don't have the same situation as 
you have in the synchronous API case anyway. You still need to handle 
async errors _as well_. It's like you resorted to having to use both the 
return value and some out parameter together to see if you have an error...

>
>> It could also fail later, as it is mostly asynchronous. Or it could succeed with a result. ie
>>
>> 	Q_SIGNAL void deviceConntected(SomeDevice device);
>> 	Q_SIGNAL void deviceConnectionFailed(SomeOtherConnectError error);
>>
>> or one signal:
>>
>> 	Q_SIGNAL void deviceConnectionResult(SomeDevice device, ConnectionResult result);
> Looking at other async APIs like QProcess and QNetworkReply having two signals isn't unusual.
True, there is something to say for both approaches. Having two signals 
means that you don't have to handle success and error conditions from 
the same method. I find that cleaner. But indeed, there is also the 
downside of having to connect to two signals, with all problems that 
entails. If you have more than two states to communicate, I think you'll 
fall back to using a single signal pretty quickly. I have given a 
job-type class a statusChanged signal, which communicates the new 
status. That may be anything from new, enqueued, waiting, cancelled, 
running, done, error, etc. Downside is that the methods dealing with 
that signal all end up as a switch statement, so there are also separate 
signals for the most important states you can connect to directly.

>
>> Also consider that there are multiple async operations, not just 'connectToDevice'.  There is scan, startSession, etc.
>>
>> Should there be a single error() covering everything?  Or one per task? Or a way to link the error back to the task?
>>
>> 	RequestId startSession();
>> 	RequestId startScan();
>>
>> 	Q_SIGNAL void error(RequestId requestId, BigSetOfErrorEnums error, QString errorStringAsWellMaybe);
> Please don't! Such APIs are ugly and hard to use. I'd consider to wrap the operations in classes and return them when starting the operation.
> Like QNetworkAccessManager does for QNetworkReply. The returned object then handles the state and emit the signals for the particular context.
> E.e. connectToDevice() could return a WiFiDevice or WiFiConnection object, which then handles the state for that particular connection.
I like that pattern too, but the ownership of the returned class is a 
problem in that situation. A shared pointer to a reply object would be 
better IMHO, or a value based object like QFuture<>. Another downside 
(for some cases) to using a reply object is that you have to connect to 
it each time you make a request, instead of once to the object you're 
making requests of. This may actually be a benefit if you have different 
places in your code where you make the requests of the same object, and 
it would be annoying or even unsafe if they all get signalled for the 
results for all requests.

Note that Qt itself is no stranger to this API either, and for simple 
cases it does have the benefit of lightness. See QObject::startTimer for 
an example. As soon as you get multiple operations that you might want 
to do on such a task/job/whatever, I'd go for a reply object though.

>
>> And there are a bunch more considerations. ie:
>>
>> - "Error" vs "Result"  (I dislike Error::NoError, prefer Result::Success)
> I think error is more consistent, but I don't have a strong opinion here.
I like Result better, for cases where there needs to be an 'OK' result 
as well.
>
>> - Is "finished()" OK if it didn't _complete_ (successfully) or should it be... "ended()" or some better word?
> finished() is commonly used also when operations succeed (e.g. QNetwork). It shouldn't be too many different signals, otherwise users will forget to connect to some of the signals.
André


-- 
You like Qt?
I am looking for collegues to join me at i-Optics!