Closed amomchilov closed 1 year ago
Hmm, I haven't experienced the same thing. This may be because I don't have very many routes and for some of my routes I want considerably different error handling behavior.
When I've written networking code using NSURL
I found the centralization of reported errors via delegates to be quite convoluted. It's true there are some benefits to it, but overall I found myself needed to work around it to "re-associate" the errors with the actual request I made in order to determine what action - if any - to take.
I'm not sure trying to address this at the framework level would be a good idea. I mean that literally, and not as indirect way of saying "I think this is a bad idea". My gut reaction is attempting to do so would introduce a new set of issues.
Adding a "global" error handler that applies across all routes of a client would be easy, but then you'd get duplicate reports of errors (once per callback, and then once again).
Indeed. And it'd be very problematic to only send errors to the "global" handler if it is set. Because then await
ed functions which experienced an error just wouldn't return and that'd terribly mess up control flow. To work around that we could instead throw an error like XPCError.alreadyProvidedToGlobalHandler(XPCError)
, but that seems deeply non-standard. (This is equally applicable to the closure-based variants, just seemed easier to describe for the async
case.)
Ideally, having a way to express "if the route callback doesn't handle the error, then give the client error handle a chance to handle it", but I don't know how we could detect if the callback handled the error or not.
I can't imagine there's any way to do that as what does it even mean to "handle the error" in any sort of deterministic sense? For example:
do {
let reply = try await client.sendMessage("Get Schwifty", route: route)
<# do something with reply #>
} catch {
if case let .other(message) = error {
print("Unable to get Schwifty because: \(message)")
}
}
In this example the error is being caught (not that I think we can detect that?), but only one XPCError
case is any way being handled - all of the rest are being ignored. I can't see how we'd able to know that.
You're right on the money, this is what I found challenging about this problem.
Here's a few different ideas I was thinking of:
It ain't pretty, but we can make a whole set of new send
overloads whose closure only expects the Success
value of the Result
(and internally the Failure
error gets forwarded to the client-level error handler). But this is restrictive, because you can't express "handle some specific error about this route, then do the global error handling relating to updating my connection status indicator (or whatever)"
I was also thinking of a more use-case-specific approach. The purpose of my shared error handler was to observe changes in the client state. Perhaps this should be a first-class feature of the client, to have a connectionStateDidChange: (ConnectionState) -> Void
callback.
Even then, that raises the question of whether route-handlers should also receive connection state callbacks (and if so, in which cases?). Hard to decide.
There's a few inter-related challenges:
If there's both a route-level and client-level error handler, we don't always want to duplicate the errors by sending them to both
myOwnSharedErroHandler(error)
manually instead.What you described here:
I found myself needing to work around it to "re-associate" the errors with the actual request I made in order to determine what action - if any - to take.
I think one thing that makes the design of this challenging for me is my ignorance on exactly when a route handler should need to know about connection statuses (this will improve as I get more real-world experience using this framework). Retries sound like one use case that would need it. Retries would a useful (essential?) feature, but IDK what architectural layer should be responsible for implementing them. For one, if we want to support the "send, oh no the Mach service isn't installed, install it, then retry" pattern, then that would required Blessed
, so perhaps retries should be implemented at the "umbrella framework" (name TBD) level?
XPCClient
API, and exposed its own public APIs that handle retrying under-the-hood. This would be a pretty heavy-handed approach, but IDK what other options there might be.Any thoughts?
For one, if we want to support the "send, oh no the Mach service isn't installed, install it, then retry" pattern, then that would required Blessed, so perhaps retries should be implemented at the "umbrella framework" (name TBD) level?
Is this a pattern you're looking to do unconditionally? Because it requires user action, that's not how my code handles things. While in theory a Mach service could become uninstalled at any time, in practice it probably will never be — the far more likely scenario is it's never been installed. So checking that on first run (or the beginning of each run) is what makes sense to me.
@amomchilov Six months later, what are your thoughts on whether client level error handling should exist?
I'll have to think more on this.
Marking unread for now, I'll come back to this when I make up my mind xD
Closing due to inactivity. If desired, please open a new issue in the future with specific examples/use cases.
One thing that I've noticed in my use for SecureXPC was that all my clients' callbacks followed the same pattern: they all
switch
on the result, and called a shared error handling function (in addition to some route-specific error handling).I think this might be a common use case, especially because changes to the connection state might want to be monitored, and that can happen on every call.
Adding a "global" error handler that applies across all routes of a client would be easy, but then you'd get duplicate reports of errors (once per callback, and then once again). Ideally, having a way to express "if the route callback doesn't handle the error, then give the client error handle a chance to handle it", but I don't know how we could detect if the callback handled the error or not.
Do you have any ideas for how we could improve this?