Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add with_Completion and with_Operation variants that can return optional values #4

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ryanslikesocool
Copy link
Contributor

There are cases where a user may want to return an optional value and not throw an error.
This PR adds NSXPCConnection.withOptionalValueErrorCompletion and other relevant function variants that can return optional values.

@mattmassicotte
Copy link
Contributor

Thank you!

Ok, I think I see the problem you need to work around. Did you think about a parameter to allow/throw on nil? I haven't thought hard about this, but just wondering which API you think is most appropriate.

@ryanslikesocool
Copy link
Contributor Author

I hadn't considered throwing as opposed to returning nil. I tend to see throwing and catching errors as "something went wrong and needs to be handled", while nil can be a perfectly valid result sometimes.

That makes me realize that some of my implementation could be simplified by catching only ConnectionContinuationError.missingBothValueAndError.

func withOptionalValueErrorCompletion(
    /* ... */
) async throws -> Value? {
    do {
        return try await withValueErrorCompletion(isolation: isolation, function: function, body)
    } catch ConnectionContinuationError.missingBothValueAndError {
        return nil
    }
}

This responsibility could be delegated to package consumers, but would require exposing the error type and an extra do/catch block for each use.

The functions could also be renamed and be overloads for the originals, but would require a @_disfavoredOverload annotation, and could cause confusion at the call site.

All that said, if the goal for the package is to provide just the essentials, I think making ConnectionContinuationError public could be enough. If the goal is convenience, I think being able to handle optional results as simply as everything else is important, whether that be by defining additional functions or figuring out another way.

@mattmassicotte
Copy link
Contributor

Ok, I completely agree with what you are saying.

I guess I'm torn. Because on the one hand, is it even a good idea to throw on nil? It is a totally valid possible value, and the current API makes it impossible to return.

I think it potentially quite convenient to have this system get rid of the optionals for you. But, that convenience now has a real cost in terms of API complexity.

What do you think about something like this?

public func withValueErrorCompletion<Service, Value: Sendable>(
	isolation: isolated (any Actor)? = #isolation,
	function: String = #function,
	// a non-optional Value
	_ body: (Service, sending @escaping (Value, Error?) -> Void) -> Void
) async throws -> Value {
	// no more nil check for value here
}

This now permits an optional generic, which I think will work for your case here? It would also shift responsibility to the body function to guard against nil values, if they are invalid. And while that is source-breaking, I don't think it's all that terrible.

What do you think about this?

@ryanslikesocool
Copy link
Contributor Author

ryanslikesocool commented Feb 27, 2025

It's definitely desired to have optionals handled when they're not expected.

I gave it a try and I don't think surfacing the optional handling to body will work, since there's no way to call the completion function (unless a default value is provided).

// service
class MyServiceImplementation: MyService { 
    func myFunction(reply: sending @escaping (Data?, Error?) -> Void) {
        var success: Data?
        var failure: Error?

        do {
            success = try someThrowingFunction()
        } catch {
            failure = error
        }
        
        reply(success, failure)
    }
}
// host
let data: Data = try await connection.withValueErrorCompletion { (service: MyService, completion: (Data, Error?) -> Void) in
    service.myFunction { (success: Data?, failure: Error?) in
        switch (success, failure) {
            case let (_, failure?): completion(/* ??? */, failure)
            case let (success?, nil): completion(success, nil)
            case (nil, nil): completion(/* ??? */, MyError.unexpectedNil)
        }
    }
}

This sure would be a lot easier if Swift.Result could be used in Obj-C protocols 😅.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants