I’d like to suggest 3 improvements to RetryScope and it’s documentation:
- NumberOfRetries is misleading -> change (at least displayname) to NumberOfTries
From the documentation:
In the Properties panel, leave the default NumberOfRetries of 3 and the Retry Interval of 5. This means that we attempt to open the Notepad window 3 times and the interval between tries is 5 seconds
Actually no, it doesn’t. In literal meaning it should read as “This means that we attempt to open the Notepad window 4 times (1 Try + 3 Retries)…”.
There is a real difference between something that was tried and something that was retried - you cannot retry something that hasn’t been tried.
If I do a RetryScope with NumberOfRetries 0 (for whatever reason, f.e. if it’s passed from configuration), it should run once. Currently it doesn’t run at all, which confirms that it’s not a NumberOfRetries, but a NumberOfTries.
This is especially important if activities in the action block have long timeouts, but in general the exact wording used does not reflect what the activity does.
It would be ideal if it could be cleaned up on a deeper level than argument display name, but that would be a breaking change.
- Add specific info in the documentation that it rethrows the last exception encountered.
This can be important if the retried action has permanent effects (any UI interaction, some IO operations etc.). The intermediate exceptions are completely discarded.
I vaguely remember it throwing an AggregateException before, but can’t pinpoint when that was changed (or why, since that was the excellent choice - “the actions failed for these reasons: …”).
Current documentation doesn’t say anything about the (actually not so uncommon case) where there can be more than 1 exception variance in the execution.
- Add an optional argument to RetryScope for an exception (or even just a message) to throw if the condition fails.
Right now it throws a completely unhelpful “System.Exception: Action failed to execute as expected.” which is wrong on 2 levels:
The message is weak (an exception occurred because something went wrong…).
It throws it as a System.Exception, which is just ugh… Non-system code should never throw System.Exception:
A better choice would be an ApplicationException (still very generic, but at least will not be confused with system meltdown). Or a CheckpointException. Or actually almost anything but a System.Exception.
Ideally if you let us specify what exception should be thrown, we can use one that is appropriate for the situation (f.e. if I’m retrying for a file to appear and it doesn’t, a FileNotFoundException would fit perfectly).