What to do with error logs that contain source information in release builds?

Question

I'm in the early stages of developing a game engine in C and so far I've been using a simple logging module to print error messages to a log file. For every possible error that could occur, I have prepared an error message that includes the file name and line number. This is all done using preprocessor macros so I can easily remove them from the release build if I desired.

Example:

size_t foobar_size = 256;
char* foobar = malloc(foobar_size);
if (!foobar)
{
    Log_Error("failed to allocate memory %d bytes", foobar_size);
    return NULL;
}

The output of this would look like:

"ERROR: failed to allocate memory 256 bytes @ src/root/main.c Line 73"

Should these error logs be included in the released version even if they contain the file name and line number of the source code? Or maybe I should keep the error messages and just remove the file name and line number, but this would make it harder fix bugs that a client reported.

I was wondering because I was able to open the executable file with any text editor and instantly see strings that look like the one above. Their is a great number of them considering that for every function with an error return value that is called their is also an error log. For some reason I felt that it was "too revealing" but at the same time it makes it very easy for clients report bugs and their exact location within the source code.

NOTE: I'm not actually planning to release anything as this is my first project and just a learning experience.

Edit: I'm not attempting to excessively hide away the internals of my program, I was asking if having file names and line numbers readable is an objectively bad idea.

Answer 1

Making your customers' lives easy

it makes it very easy for clients report bugs and their exact location within the source code

Nope. Reporting bugs is not the job of your customers; it's your job. They are using your app to play games, to have fun. Searching for your contact info to report a bug by copy-pasting some cryptic string is all but fun.

It's all but easy as well. When I must spend ten minutes searching for contact info and finding that I need to register on some bulletin board built in nineties and never updated since then, and when I waste even more time to do that, I receive a reply telling that my bug report was closed, because it doesn't follow some stupid format, it encourages just one thing: go use competitor's app instead.

Look at applications such as Chrome or Firefox. When they crash, do they show cryptic stuff? Not really. They apologize, kindly explain what happened, and send technical info in the background.

Other applications, such as VLC, seem to be more focused on privacy than on user experience, so they do things slightly differently: they don't silently send info to some distant server; instead, when they crash, they apologize, kindly explain what happened, and present a button which would send technical info.

Blindly showing something such as:

ERROR: failed to allocate memory 256 bytes @ src/root/main.c Line 73

and let the users do your job is just... insulting.

Moreover, it's a technical error. it makes no sense to show technical errors to users, because they are not expected to know what the memory is, why is there exactly 256 bytes, and what allocating memory implies exactly.

A bit of theory

In a case of an unrecoverable error, there are three concerns:

• Ensure there is no data loss (for instance, it would be nice if a word processor, in a case of a crash, would attempt to save a document I was working on for the past two hours).

• Report the error to developers in order to handle the situation which led to the error.

• Help the user understanding what happened, either in order to avoid the situation to happen again during the time the bug is fixed, or in order to fix what led to the error, if the user considers that he's technically skilled and has enough time to waste fixing stuff.

Error messages are irrelevant for the first concern.

When it comes to the second one, technical error messages are relevant. As explained above, they should be sent to developers eventually with the permission of the user, but without requiring any other action but permission. Those reports would usually contain a stack trace, the version of the application, etc.: barely including the file name and the line number is usually not enough.

The third one is more interesting. Many applications mistakenly use technical error messages here; however, users don't care about technical error messages. This applies independently of the technical skills of the user. Another day, Visual Studio refused to do an action telling me that “Object reference not set to an instance of an object.” While I'm a developer, I couldn't care less about Visual Studio's objects, instances and references. What was important is that the app failed to do what I asked it to do.

Understanding that, recently, applications adopted a different behavior. Some started presenting generic, user-oriented error messages, such as:

No internet connection.

While they may give a hint in some cases, in some others, they are unhelpful and rude by their shortness (what makes the app A on my mobile device telling me that there is no internet connection, while five other apps connect to the internet successfully?).

Others pushed even further, and just stopped showing any error. For example, when iTunes fails to synchronize data with an iPad, it simply and silently stops synchronizing, pretending that everything went fine.

There is, however, a much better way of handling an abnormal situation. An app which has some respect for its users would apologize, explain, in non-technical terms, what happened, and give some pointers to what could help searching for more information. Many Linux services work this way, by giving a error number which would help finding a lot of resources on Stack Overflow and ServerFault about ways to solve or circumvent the problem. Although error numbers are not very user friendly, their uniqueness is very valuable, and makes errors much more searchable compared to a “Object reference not set to an instance of an object.”

Revealing the internals of your app

For some reason I felt that it was "too revealing"

And so? It's not banking software that has some innovative way of computing the risk of working with derivatives. It doesn't look like all your competitors would rush with disassemblers onto your code that you are... “not actually planning to release” anyway.

Showing the line number in the logs, for instance, has nothing wrong for such software. You make your job much easier, while introducing no major drawbacks.

There are things which should never figure in logs, such as users' passwords or credit card numbers, or virtually any personal information. Line numbers (and, more generally, stack traces) are not among information which should be hidden.

If you work on software which should be kept safe against competitors, then, indeed, it would make sense to hide both filenames and line numbers. In this case, it would be possible to use arbitrary error numbers, which, while remaining eventually cryptic for your competitors, would allow you to map them to source code. Those cryptic numbers would have two purposes: be included in automatic bug reports and logs, and help users identify what to do in a case of a error.

Breaking the rules

it seemed really awkward to have that info readable in the binary. I was afraid of breaking some kind of unspoken-rule

When compiling an app, strings in general are usually stored as... well... strings (some Unicode ones would be quite unreadable, however). Note that what you see by opening your executable in a text editor is not that interesting nor relevant; someone who wants to study your app will use a decompiler which will show not only the strings (including Unicode ones, this time), but also other hardcoded values and even the structure of your app with its conditional statements, loops, and so on.

Unless you intentionally hide the internals of your app by using obfuscators, the fact that anyone could explore your app is perfectly acceptable. Although it may be scary to know that someone would be able to guess how your app was built, you shouldn't care about that too much. If you're a beginner programmer, you should care much less about hiding stuff and much more about making your personal projects open source, which would also help you presenting them to potential recruiters.

Don't be scared, you don't break any unspoken rules by letting strangers a glimpse in the internals of your app.

Answer 2

The fundamental problem you're having is that you need the precise location in your source where an error occurred but don't want information about the structure of your software in the hands of your end users. I'm not here to pass judgment on the merits of that need or how the information gets from your program back to you, because that's entirely your decision.

What you're after is a variation on opaque pointers. Instead of providing the actual information, provide a reference that you can use to identify the location later. One way to do this is by assigning a number to each error report that's incremented each time you code a new one:

Log_Error(6060842, "Failed to get the line.");

Log_Error(8675309, "Unable to locate Jenny.");

When you get a report of error 8675309, you have to associate it with its location in the source, which is done easily enough by searching it:

% cd /path/to/sources
% fgrep -rn 'Log_Error(8675309,' .
src/root/main.c:73:Log_Error(8675309, "Unable to locate Jenny.");

One advantage to this approach is that the reference number stays with the code even if it moves to a different file, disappears entirely if it's deleted (numbers are never re-used) and can be used to avoid a wild goose chase if someone tells you he was running version 1.5 but was actually running 1.4 and there's now a different bit of error code in the same location.