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

Incorrect usage of throw in operation #222

Closed
thebarndog opened this issue Mar 30, 2016 · 1 comment
Closed

Incorrect usage of throw in operation #222

thebarndog opened this issue Mar 30, 2016 · 1 comment
Assignees

Comments

@thebarndog
Copy link
Contributor

In #216, I mentioned about adding throws to the operation block for ReactiveExtensions. However you added throws in the wrong place so that the signature of operation is now:

 func operation(operation: (context: Context, save: () throws -> Void) -> Void)

when it should be:

func operation(operation: (context: Context, save: () -> Void) throws -> Void)

In the first, and currently released version, the throws is on the save block but the save block doesn't and shouldn't throw. The docs clearly state that the save parameter is called like save but if save throws then it has to be called as try save, inside an operation block that ALSO throws.

Take the following example:

return self.storage.operation { context, save in
            try context.insert(model)
            save()
        }

Because save is marked as throws now, the compiler errors out saying can call throw, but it not marked with 'try' and the error is not handled. And since the operation block doesn't throw, try context won't work because it's inside a non-throwing closure.

What I think you want is for the save block AND the operation block to throws, which would result in:

func operation(operation: (context: Context, save: () throws -> Void) throws -> Void)

Then the consumer of the operation function would use it like:

    storage.operation { context, save in
         try context.insert(model)
         try save()
    }

where the error thrown would be caught by the operation block.
In RealmDefaultStorage, that would look something like:

public func operation(operation: (context: Context, save: () throws -> Void) throws -> Void) throws {
        let context: Realm = self.saveContext as! Realm
        context.beginWrite()
        var save: Bool = false
        do {
            try operation(context: context, save: { () throws -> Void in
                defer {
                    save = true
                }
                try context.commitWrite()
            })
        } catch {
            context.cancelWrite()
            throw error
        }

        if !save {
            context.cancelWrite()
        }
    }
@pepicrft
Copy link

pepicrft commented Apr 5, 2016

@startupthekid thanks for reporting it. I agree with you. These are the changes I introduced on this #223 PR:

  • Operation closure now throws. Developers can use context operations that throw using just try without catching the error inside the operation closure.
  • If there's an error thrown inside that closure, either because of a developer operation or the save block throws it, this is going to be forwared up to the operation throwing.

In case of the reactive interface. When something throws an error, it's sent as an error through the Reactive stream.

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