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

Print useful error messages in release builds #3382

Closed
jordo opened this issue Oct 4, 2021 · 4 comments
Closed

Print useful error messages in release builds #3382

jordo opened this issue Oct 4, 2021 · 4 comments
Milestone

Comments

@jordo
Copy link

jordo commented Oct 4, 2021

Describe the project you are working on

Godot

Describe the problem or limitation you are having in your project

Useful information regarding error messages aren't printed in release builds. This makes fixing problems in production environments difficult.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Currently, error message information is stripped in release builds, with this commit:

godotengine/godot@0d7409a

What this does, is substitute an empty string for the additional error information you would normally see printed in output.

For example, we have a config file error in a production game, and this is what we see:
Screen Shot 2021-10-04 at 10 46 52 AM

In other builds, we would see what section and key the error comes from here:
Screen Shot 2021-10-04 at 10 47 29 AM

This is due to the fact that in release builds, the additional error information isn't printed out. I can't understand why this is the case, as it makes debugging and fixing production games more difficult.

The only reasoning for this I can think of is a potential security issue, but that shouldn't be considered because any parameters passed to any error macro here already has the arguments in memory, which means they can be read by simply looking at memory. (Not printing them out, isn't actually more secure, it just makes reading this information potentially more difficult)

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Revert the commit godotengine/godot@0d7409a

I think the majority of developers with production games would appreciate the additional logging information in any bug reports they receive.

If this enhancement will not be used often, can it be worked around with a few lines of script?

It cannot be worked around, as it's in core error_macros.h

Is there a reason why this should be core and not an add-on in the asset library?

It's core, as defined in error_macros.h

@YuriSizov
Copy link
Contributor

The commit you've linked only restored the intended behavior. It was removed previously by mistake. The PR this commit comes from has an answer to your "Why", by the way: godotengine/godot#33595

@jordo
Copy link
Author

jordo commented Oct 4, 2021

From discussion in rocket chat today, the reasoning was for omission was string allocation/overhead considerations and binary size considerations.

Our wasm build size with these error messages enabled and disabled is below:

27,326,350 bytes vs 27,242,926 bytes. An increase of 81kB in binary size, so adding these errors message details resulted in a 0.3% increase in binary size for our web builds.

Looking at macro expansions, any other processing only happens if the conditional passes.

Screen Shot 2021-10-04 at 11 32 43 AM

@Calinou
Copy link
Member

Calinou commented Oct 4, 2021

Given the relatively small increase in binary size, I think it's worth keeping messages in release builds. The compressed size will likely increase even less as strings can be compressed fairly well.

@YuriSizov
Copy link
Contributor

Closed by godotengine/godot#53405

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

3 participants