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

Proposal: error handling with generic parameter #39

Closed
gloooooooomy opened this issue Apr 25, 2018 · 5 comments
Closed

Proposal: error handling with generic parameter #39

gloooooooomy opened this issue Apr 25, 2018 · 5 comments

Comments

@gloooooooomy
Copy link

Hello :)

Idea : make a overloaded catch function with generic parameter

extension Promise {
    
@discardableResult
public func `catch`<ErrorType: Error>(on queue: DispatchQueue = .promises, _ reject: @escaping (ErrorType) -> Void) -> Promise {
    return self.catch(on: queue) { (error) -> Void in
        guard let typedError = error as? ErrorType else {
            return
        }
       reject(typedError)
   }
}

and using like

promiseFunction().catch { (error: MyError) in
    // error is MyError
}.catch { error in
    // default
}

instead of

promiseFunction().catch { error in
    if let myError = error as? MyError {
        // handling error
    }
}
@shoumikhin
Copy link
Contributor

Looks cool :)
Do you propose to have it alongside the existing catch operator as an overload?
I'm afraid it will interfere with the existing Error-only catch, and the complier would silently pick one overload over another, or complain unless you clearly state the error type.
Anyhow, pull requests are more than welcome!

@gloooooooomy
Copy link
Author

Yes, i proposed catch operator as an overloaded function.
because compiler could infers the generic type only when state the error type.

.catch { (error: MyError) in
  // compiler picks generic catch operator.
}
.catch { (error: Error) in
  // compiler picks default catch. because generic type does not supports protocol.
}
.catch { error in
  // same as "error: Error"
}

also i made a pull requests, please review :)

@shoumikhin
Copy link
Contributor

shoumikhin commented Apr 27, 2018

@gloooooooomy thank you, I love your proposal!

Just have a concern with the case like the following:

enum CustomError: Error { case test }

Promise<Void> {
  throw Test.Error.code42
}.then { _ in
  XCTFail()
}.catch { error in
  XCTAssertTrue(error == CustomError.test)  // Never executed.
}

I imagine, it's expected that the catch above to be executed w/o specifying the error type explicitly, since it must catch all Errors. Unfortunately, it's not, because the compiler translates that one-liner catch as your generic version and derives the type based on the condition in the test assert. I.e. it thinks the catch generic type must be CustomError. But Test.Error is thrown, in fact. So that catch is never executed, since types don't match. To fix that behavior, one should either specify the expected error type explicitly as (error: Error) in(which is not always convenient and can be omitted by mistake), or write more than one line of code inside the catch closure to force the compiler to pick a more suitable non-generic catch overload (and that is potentially error-prone, because the clients may not be aware about such subtle implications).

Ideally, we'd want something like default generic arguments be implemented in Swift to have only one version of catch, and actually make it generic, as you've proposed.

What do you think?

@shoumikhin
Copy link
Contributor

@gloooooooomy let's close this issue for now, but don't hesitate to follow up if you have any ideas on how to deal with the current compiler limitations.

@gloooooooomy
Copy link
Author

@shoumikhin sorry for reply too late. :(
now i know exactly what you concerned. and I'll check if there's another way to avoid.

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

No branches or pull requests

2 participants