On Tue, Aug 25, 2009 at 4:03 PM, Aaron Boodman <[email protected]> wrote:
> Thanks for the feedback, James. > > Part of the motivation for this design was that errors are (or at > least should be) rare, and so we should not overoptimize the paths for > handling them. Hopefully error will be vanishingly rare, but some errors are unavoidable. Errors that are rare also tend to be less likely to show up in the development cycle which makes it more tempting to not code for error paths and blow up horribly in the wild (I didn't catch the chrome.tabs.create() error condition until trying on a different machine). > > In the case of your bug, we should fix the bug, and there shouldn't be > cases like this where developers need to retry. > > I feel like having an explicit error handler could work too though. > The main reason I'm opposed to it is that it makes the signatures in > the documentation longer when in most cases it is unnecessary > information. I think if it was a consistent pattern that every async call's last parameter was an object with two optional callbacks, one with the real parameters and one with the error, then it should be pretty easy for a developer to grok after seeing it once or twice. - James > > > - a > > On Tue, Aug 25, 2009 at 12:55 PM, James Robinson<[email protected]> wrote: > > I've had a chance to play with some failing async API calls and have to > > admit I don't really like the chrome.extension.lastError pattern. The > basic > > issue is that after an async call completes either successfully or with > an > > error the same callback is invoked. However if there was an error then > the > > parameters on the callback are invalid and the callback has to use the > > 'lastError' variable as the logical parameter. In other words, the start > of > > every callback has to look something like this: > > function fooCallback(param1, param2, param3) { > > if (chrome.extension.lastError) { > > // Don't use param1, param2, param3, instead use > > chrome.extension.lastError to try to recover > > } else { > > // Now we can use param1, param2, param3 > > } > > } > > This boils down to the original proposal of having a 'success' flag be > the > > first parameter except that it's a rather awkward way to go about it. > > Invariably when writing my own code I forget to check > > chrome.extension.lastError in my callback and die horribly at the first > > attempt to reference a parameter on the callback. If they do something > like > > store a reference to the callback parameters in a container for later > > processing then the undefined reference might not show up until much much > > later and be impossible to track down. In cases where the callback is > > parameterless (for example chrome.tabs.update()) an extensions developer > > will typically forget to check lastError and proceed with their > application > > logic regardless of if the call succeeded. This seems less than ideal :( > > The second issue with the chrome.extension.lastError approach is that > > typically by the time my callback was invoked it was too late to handle > the > > error in any reasonable way. In my case the underlying source of the API > > failures was http://crbug.com/13284 and the workaround was to retry the > call > > a bit later. In order to retry the call I had to keep all the parameters > to > > the original call around somewhere and have them available in the success > > callback. The pattern looked something like this: > > var createParam = { ... }; > > chrome.tabs.create(createParams, function(tab) { > > if (chrome.extension.lastError) { > > handleTabCreatedError(chrome.extension.lastError, createParams); > > } else { > > realTabCreatedCallback(tab); > > } > > }); > > This gets a little verbose to have on every asynchronous API call. > > I would suggest having explicit successCallback / errorCallback callbacks > > set on an object sent in to each async call. This way the extension > writer > > can keep their error handling code separate from their normal application > > logic, avoid accidentally treating an error callback as a success > callback, > > and be able to use only the parameters to the callbacks rather than > globals > > sitting on chrome.extension. chrome.tabs.create's signature would then > look > > like: > > void create([{[string url], [int windowId], [int index], [bool > selected]}], > > [{[void successCallback(Tab)], [void > > errorCallback(Error)]}]); > > successCallback() would only be invoked on a successful completion of the > > call and the Tab parameter would always be a valid chrome.tabs.Tab > object. > > errorCallback() would be invoked if the call failed with an Error > parameter > > which would replace the chrome.extension.lastError object. > > - James > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Chromium-extensions" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/chromium-extensions?hl=en -~----------~----~----~----~------~----~------~--~---
