Using NSLog to log errors

[peternlewis]

In my app, I log errors to a file (and optionally also to the console). Since the console is so ridiculously cluttered with noise these days, logging to a log file makes finding information a lot easier.

FMDB just logs using NSLog. I am looking at replacing this with a call to a setable block that takes a string. By default, the block would simply call NSLog, but in my code it could redirect it to a file.

Looking through the code (mostly just FMDatabase.m currently, but that seems to be the vast bulk of the NSLog's), there are various NSLog's that are not “protected” by the _logsErrors boolean. So it is possible that really the block needs to take two parameters, an “alwaysLog” flag and a string error message.

Further, there is a lot of inconsistency and duplication of code in the error reporting. Some use "DB Error:", some do not have the prefix "DB", and there are a bunch of places where the query and/or path. So if I did this, I would probably provide some helper functions so those queries are consistent. I'd probably put "FMDB" at the start of all the error messages for clarity.

My code is somewhat bastardised from the stock FMDB since I run in Objective-C++ with all errors on, but if it would be helpful, I'm happy to feed back the work on this. Any thoughts?

[ccgus]

It's not a bad idea. Some random thoughts:

What if the block were a static variable on FMDatabase? I wonder if that would screw up folks who have multiple instances of it?

We could change all the if (_db.logsErrors) NSLog(@"%@", errorMessage); instances to something like [FMDatabase logError:format, …]; and then that would check for the existence of the block or then go to stdout. But that would change the signature from an instance var to a class, so maybe it shouldn't be static…

Anyway, sure, go for it. Just no breaking the interface until FMDB 3, which is probably 4 years late at this point.

[peternlewis]

OK, here is a diff of what I implemented. As far as API, it only added a few lines to the header, one to define the block typedef, and another two to define static global custom log block setter and another to define an instance custom log block setter/getter. The code uses the instance-specific one first, failing that the global one, failing that just NSLog. I also tidied up some of the error message consistency, and added code to convert the rc code to a string for the typical cases (could be extended to all cases if desired). I did make the "Save points require SQLLie 3.7" always reporting rather than only for logsErrors, since it seems to me that you really need to hear about that issue. Feel free to adjust as desired.

I tried to stick to the code style, but its quite different to mine so I don't know how well I succeeded. Also I run with Objective-C++, but I don't think I did anything that would conflict with just Objective-C.

fmdb-logging.txt

[ccgus]

Danke! I'm about to go on vacation for a bit, but I'll try and check it out when I get a chance. Might not be for a few days though.

[peternlewis]

No worries, enjoy your holiday.

[ccgus]

I tried running patch against your diff, and got a bunch of errors:

patching file FMDatabase.h
Hunk #1 succeeded at 76 (offset 1 line).
Hunk #2 succeeded at 101 (offset 1 line).
patching file FMDatabase.m
Hunk #1 succeeded at 8 with fuzz 2 (offset 4 lines).
Hunk #2 succeeded at 177 with fuzz 1 (offset 13 lines).
Hunk #3 succeeded at 211 (offset 23 lines).
Hunk #4 succeeded at 224 (offset 25 lines).
Hunk #5 FAILED at 251.
Hunk #6 succeeded at 257 (offset 24 lines).
Hunk #7 succeeded at 294 (offset 26 lines).
Hunk #8 succeeded at 328 (offset 26 lines).
Hunk #9 succeeded at 428 (offset 31 lines).
Hunk #10 succeeded at 507 (offset 31 lines).
Hunk #11 succeeded at 521 with fuzz 2 (offset 31 lines).
Hunk #12 FAILED at 538.
Hunk #13 succeeded at 878 with fuzz 1 (offset 31 lines).
Hunk #14 succeeded at 892 with fuzz 1 (offset 31 lines).
Hunk #15 succeeded at 918 (offset 31 lines).
Hunk #16 succeeded at 933 (offset 31 lines).
Hunk #17 succeeded at 954 (offset 31 lines).
Hunk #18 succeeded at 968 (offset 31 lines).
Hunk #19 succeeded at 1060 with fuzz 1 (offset 30 lines).
Hunk #20 succeeded at 1074 (offset 30 lines).
Hunk #21 succeeded at 1106 (offset 30 lines).
Hunk #22 succeeded at 1124 (offset 30 lines).
Hunk #23 succeeded at 1149 (offset 30 lines).
Hunk #24 succeeded at 1165 (offset 30 lines).
Hunk #25 succeeded at 1185 (offset 30 lines).
Hunk #26 succeeded at 1200 (offset 30 lines).
Hunk #27 succeeded at 1237 (offset 30 lines).
Hunk #28 succeeded at 1319 (offset 31 lines).
Hunk #29 FAILED at 1421.
Hunk #30 FAILED at 1434.
Hunk #31 FAILED at 1447.
Hunk #32 FAILED at 1478.
6 out of 32 hunks FAILED -- saving rejects to file FMDatabase.m.rej

Want to send a PR, or update to the latest trunk for me to diff against?

[peternlewis]

Not overly surprising - as I mentioned, my version is quite bastardised from the original because of the necessary changes to make it work with Objective-C++ -Weverything.

Probably that and I don't run on the latest version (because of the former, I rarely update it, it is too time consuming to re-apply the necessary changes each time).

I will look at resolving the diff against the current version when I get a chance.

[peternlewis]

OK, here is an updated patch which should work against the current .m/.h. Of course this isn't actually the version I am using so I can't actually test it (I can't even compile it actually). fmdb-logging2.txt