Closed amomchilov closed 2 years ago
I agree with the problem. Will leave feedback on the PR in regards to the specific solution.
Response to https://github.com/trilemma-dev/SecureXPC/pull/18/files#r748837217
Hmmm you're totally right.
Seems like a way to ensure consistency here is under the hood to always send XPC messages that expect a reply and then have the send(...) functions that don't have a reply be able to provide an error handler. Or potentially as a bonus make it a completion handler so that it's possible for the client to get confirmation the message was received.
I experimented with this idea. One thing to note is that you can't just use a Result<(), XPCError>
reply type as usual (because Void
isn't Codable
).
Or potentially as a bonus make it a completion handler so that it's possible for the client to get confirmation the message was received.
I think this would be good. The completion handler can be ((XPCError?) -> Void)?
. When it's nil
, we can optimize and use the reply-less API.
Aside from the consistency issues, the approach in this PR makes it impossible (or at least inconvenient?) for the client to know which send(...) calls didn't succeed.
True. I didn't notice this because I was narrowly interested in seeing the connected/disconnected status of my connection (I built a small little debugging UI in my App using NSImageNameStatusNone
/NSImageNameStatusAvailable
/NSImageNameStatusUnavailable
indicators)
I experimented with this idea. One thing to note is that you can't just use a Result<(), XPCError> reply type as usual (because Void isn't Codable).
Why does the reply type need to conform to Codable
in this case?
Right now the reply handler is public typealias XPCReplyHandler<R> = (Result<R, XPCError>) -> Void
which has no requirement the success type be Codable
. The send(...)
and sendMessage(...)
functions that take replies have a constraint of R: Decodable
, but the two functions without replies wouldn't need to. Also we don't have to use this same typealias (and if we do, we should rename it to something more generic so it doesn't only make sense for replies.
I think this would be good. The completion handler can be ((XPCError?) -> Void)?. When it's nil, we can optimize and use the reply-less API.
Hmm, not sure we should actually allow it to be nil
considering that makes it easy to ignore thrown errors. Seems rather against the Swift's overall error handling paradigm.
@jakaplan
Why does the reply type need to conform to Codable in this case?
Although the reply-less send
/sendMessage
wouldn't have the R: Decodable
constraint, they both end up forwarding to sendWithReply
, which does. Unless we added a new send method which takes care of it.
Seems rather against the Swift's overall error handling paradigm.
On the contrary, Swift has a whole suite of "escape hatches" (!
, try!
, as!
, unsafeBitCast
, etc.). I haven't nailed down the exact design for my app <-> helper communications, but one part of it has a UDP-like quality: if the message didn't make it the first time, it's too late to resend it (it would be useless if it's late), and there's not much error handling I could do.
Although the reply-less
send
/sendMessage
wouldn't have theR: Decodable
constraint, they both end up forwarding tosendWithReply
, which does. Unless we added a new send method which takes care of it.
I imagine it'd be a function which "wraps" the existing sendWithReply
functional or potentially this is directly done inside of the send
and sendMessage
functions depending on how much code is involved.
On the contrary, Swift has a whole suite of "escape hatches" (!, try!, as!, unsafeBitCast, etc.).
Those don't let you ignore the error though, they terminate your process if an error actually occurs. If I understood you correctly passing in nil
would mean nothing occurs on the client in response to an error being thrown by the server.
I imagine it'd be a function which "wraps" the existing sendWithReply functional or potentially this is directly done inside of the send and sendMessage functions depending on how much code is involved.
I'm looking into it. First I was trying to decide if this warrants new route types.
If I understood you correctly passing in nil would mean nothing occurs on the client in response to an error being thrown by the server.
That's pretty much equal to _ = try? somethingThatMightThrow
That's pretty much equal to try? somethingThatMightThrow (where the resulting value is not used
Good point, I so rarely use try?
I forgot it exists. Actually while I've never done this before, it turns out try?
can also be used when calling a function that has no return value. This is perfectly valid:
let client = XPCClient.forMachService(named: "com.example.service")
let route = XPCRouteWithoutMessageWithoutReply("configamajig", "discombobulate")
try? client.send(route: route)
So with that in mind, sure passing in nil
is fine to ignore any error.
When there's no "reply" block, there's nowhere to detect errors that were encountered after sending a reply-less message.