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

continueOnError issue #35

Open
AgentFeeble opened this issue Aug 3, 2016 · 5 comments
Open

continueOnError issue #35

AgentFeeble opened this issue Aug 3, 2016 · 5 comments

Comments

@AgentFeeble
Copy link

The continueOnErrorWith(continuation:) method currently requires that a task returning the same type of result as the receiver be returned in the continuation closure. This makes something like this impossible:

let task: Task<Settings> = getSettingsTask()

let continueTask = task.continueOnSuccessWithTask { (settings: Settings) -> Task<String> in
    return self.updateSettingsTask()
}

task.continueOnErrorWith { (error: ErrorType) -> () in <-- compiler error here
    self.showError(error)
}

This produces the compiler error Declared closure result '()' is incompatible with contextual type 'Settings', because the continueOnErrorWith(continuation:) method expects the closure to return a Settings instance. A similar issue arises with the continueOnErrorWithTask(continuation:) method.

A less than ideal workaround is to do this

task.continueWith { (task: Task<Settings>) -> () in
    return
}.continueOnErrorWith { error in
    self.showError(error)
}

A proposed fix is to change the method signatures to

public func continueOnErrorWith<S, E: ErrorType>(executor: Executor = .Default, continuation: (E throws -> S)) -> Task<S>
and
public func continueOnErrorWithTask<S, E: ErrorType>(executor: Executor = .Default, continuation: (E throws -> Task<S>)) -> Task<S>
@nlutsenko
Copy link
Member

That's a great suggestion, but there is a weird scenario here, that I would love to discuss.

Taking into account that continueOnErrorWith doesn't run at all times, and only runs when the task is faulted - returning a non TResult from it is making an entire chain of tasks behave in a weird way, that is allowed by the type system at that point, but looks very weird and strange.

Let me give you an example:

let continueTask = task.continueOnSuccessWithTask { (settings: Settings) -> Task<String> in
    return self.updateSettingsTask()
}.task.continueOnErrorWith { (error: ErrorType) -> () in
    self.showError(error)
}

The type of continueTask is Task<Void> and not Task<Settings>, but if the entire chain succeeds and actual result of continueTask is of type Settings and not Void.

At the current moment continueOnErrorWith is built for error recovery, where it still doesn't violate the type system and the final result of the chain of tasks.

cc @grantland @richardjrossiii for discussion

@nlutsenko
Copy link
Member

We have makeVoid() in Bolts-Android, and looks like it might be a great candidate for this scenario, where you won't have to create an empty continuation just to cast the task to Void result.

@AgentFeeble
Copy link
Author

I always thought of the error handler as being at the end of the chain, so I didn't consider that situation. I think that putting the error handler at the end of the chain is probably the way its used most of the time, but I agree that enforcing this restriction at the framework level isn't great.

I can also see the workaround I proposed suffers from the same issue. I don't see how the current implementation solves it though, without the makeVoid method. Is the error handler responsible for generating the correct result?

I like the idea of makeVoid(). How does it work though? Is the result of the previous task in the chain passed on to the error handlers continuations? The only problem with that is, we don't know which task in the chain failed, and a result might not have been created yet. The only way I see it working is if the error handler generates the result. Am I right in thinking this way?

@AgentFeeble
Copy link
Author

I still feel like my proposed solution provides the flexibility to be able to switch to a void task, or return a result of the same type as the previous task in the chain. The swift compiler is smart enough to pick up any type mismatches. But this way the developer will have the flexibility to decide which way to use it.

Or Is the explicit makeVoid call to opt in to the functionality preferred over the implicit switch? The more I think about it, the more it seems like its just a matter of style preference, explicit vs implicit

@jkmathew
Copy link

jkmathew commented Feb 8, 2018

Hi @nlutsenko , I think makeVoid()'s counter part in swift is emptyTask(). But it is internal function. How we can make youse of internal functions in our own modules.

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

No branches or pull requests

3 participants