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

Throw an error occurred inside Query.pluck() #470

Merged
merged 2 commits into from
Jul 21, 2016

Conversation

tasanobu-zz
Copy link
Contributor

I fixed the issue reported in #414 to avoid app crash.

@@ -955,7 +955,10 @@ extension Connection {
}

public func pluck(query: QueryType) -> Row? {
return try! prepare(query.limit(1, query.clauses.limit?.offset)).generate().next()
guard let rows = try? prepare(query.limit(1, query.clauses.limit?.offset)) else {
return nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably propagate the error up instead of returning nil.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So basically I think this should be

public func pluck(query: QueryType) throws -> Row? {
  return try prepare(query.limit(1, query.clauses.limit?.offset)).generate().next()
}

@tasanobu-zz tasanobu-zz changed the title Prevent app crash which is caused by an error thrown in Query.pluck() Throw an error occurred inside Query.pluck() Jul 20, 2016
@tasanobu-zz
Copy link
Contributor Author

@nickmshelley
I’ve changed the implementation to propagate an error instead of returning nil

@nickmshelley
Copy link
Collaborator

Thanks @tasanobu!

@stephencelis I'd love to see this merged when you get some time.

@hiltonc hiltonc merged commit d721d29 into stephencelis:master Jul 21, 2016
@nickmshelley nickmshelley mentioned this pull request Sep 8, 2016
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.

4 participants