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

Consider using fatalError instead of precondition #37

Closed
robertjpayne opened this issue Mar 31, 2016 · 9 comments
Closed

Consider using fatalError instead of precondition #37

robertjpayne opened this issue Mar 31, 2016 · 9 comments

Comments

@robertjpayne
Copy link

Because GRDB is a dependency, when installed via Carthage it's compiled under release mode which means the precondition messages are lost.

I've found it sort of painful to do a debugging dance with GRDB ( have to re-compile with --configuration Debug in carthage ) just to get the precondition error messages to figure out what programmer error I've made.

If you instead had a custom "check" method that fired a fatalError instead of a precondition it would:

  1. Ensure that undefined and programmer error behaviour can never be released (-Ounchecked would skip these)
  2. Ensure via Carthage these errors are printed to the console.

Curious what other libraries are doing to. In SnapKit we're ensuring we use fatalError's.

@groue
Copy link
Owner

groue commented Mar 31, 2016

Hello @robertjpayne

Your first sentence contains something wrong:

it's compiled under release mode which means the precondition messages are lost.

Quoting the documentation of precondition():

Use this function to detect conditions that must prevent the program from proceeding even in shipping code.

In -O builds (the default for Xcode's Release configuration): if condition evaluates to false, stop program execution.

In -Ounchecked builds, condition is not evaluated, but the optimizer may assume that it would evaluate to true. Failure to satisfy that assumption in -Ounchecked builds is a serious programming error.

My own tests confirm this behavior: an app that loads a GRDB framework compiled with Carthage, and generally compiled in Release mode, does trap on GRDB's preconditions.

I've found it sort of painful to do a debugging dance with GRDB

Well, maybe you have some other problem? Could you share one of those errors that get lost? Maybe you don't have a precondition problem, but with one of GRDB's assert() (asserts get lost in Release builds)?

Curious what other libraries are doing to. In SnapKit we're ensuring we use fatalError's

Well, I hope I have convinced you that there is nothing curious libraries are using preconditions. They are using the dedicated tool for the job.

Tell me the results of your investigations.

@robertjpayne
Copy link
Author

@groue

Sorry to clarify the preconditions are still firing but only their messages are being lost. In release mode the linked framework ( Carthage integration here at least ) breaks and has a bit of a stack trace but no error messaging which is a bit painful.

In SnapKit we're using fatalError's specifically to ensure the messages don't get lost and the user end up with just random break points during development.

The obvious fix here is to compile frameworks with DEBUG=1 during development but Carthage doesn't make that so simple.

@groue
Copy link
Owner

groue commented Mar 31, 2016

OK @robertjpayne, I get your point: I don't see precondition messages in my crash logs.

😞 That's more an issue with Swift, actually... Did you open an issue at http://bugs.swift.org ?

But I'm not there to blame Swift and leave the library users naked until this gets fixed. Let me investigate this a little further, please.

@robertjpayne
Copy link
Author

@groue yea it's definitely more a Swift issue I agree I should log a bug as it's unhelpful that these messages are missing :(

I didn't open an issue yet I probably should!

@robertjpayne
Copy link
Author

@groue
Copy link
Owner

groue commented Mar 31, 2016

Thanks @robertjpayne for the link. All right, let's switch to fatalError, then!

@groue
Copy link
Owner

groue commented Mar 31, 2016

By the way, let me ask you a favor:

I'm wondering the kind of precondition failures you get. I've tried to make GRDB as foolproof as possible, and I intend to make it difficult to fire preconditions. Since you have some, there may be room for improvement. But I need to know the kind of errors you get. Would you please, if you have a few minutes, tell me a little more?

@groue groue closed this as completed in 06095ec Mar 31, 2016
@robertjpayne
Copy link
Author

@groue this was a very simple one:

self.db.inTransaction { db in
   self.db.execute(…)
}

This fires the re-entrant error and is very easy to make, the failure message made it super obvious once it happened but otherwise was difficult to sort it out.

@groue
Copy link
Owner

groue commented Mar 31, 2016

Thank you @robertjpayne, for the details, and for opening this issue: What's the purpose of writing clear error messages if they can't be read by the user, hu? 😄

You can update to GRDB v0.54.2: it has shipped with the explicit fatal errors that work around https://bugs.swift.org/browse/SR-905.

Happy GRDB!

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

2 participants