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

SEVERE BUG IN LOGKITTEN -- INCORRECT LOGGING #207

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

leijurv
Copy link
Contributor

@leijurv leijurv commented Jan 6, 2018

Leaving logMessage(Object message, KittenLevel level, boolean override) as public introduces a subtle bug.

private static String getLoggerMethodCallerMethodName() {
	return Thread.currentThread().getStackTrace()[4].getMethodName(); // caller of the logger method is fifth in the stack trace
}

This method gets the fifth item in the stack trace. Normally, a class calls, for example void f(Object message) { // Log fatal message, which then calls LogKitten.logMessage(message, KittenLevel.FATAL, false);. This results in the fifth item in the stack trace being, as expected, the function that calls LogKitten.

However, if logMessage were public, the fifth item in the stack trace would be the function that calls the function that calls logkitten, resulting in incorrect logging.

The levels of indirection in calling LogKitten must be standardized, otherwise logging of what function called LogKitten will be inconsistent.

@oldgalileo oldgalileo self-requested a review January 7, 2018 05:32
@oldgalileo oldgalileo assigned andzerb and unassigned andzerb Jan 7, 2018
Copy link
Contributor

@oldgalileo oldgalileo left a comment

Choose a reason for hiding this comment

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

Please test and ensure that the logging format is the same. Then will merge.

@leijurv
Copy link
Contributor Author

leijurv commented Jan 7, 2018

Logging will not be the same, that's the point. This changes the logging.

@leijurv leijurv changed the title Update LogKitten.java SEVERE BUG IN LOGKITTEN -- INCORRECT LOGGING Jan 9, 2018
@leijurv
Copy link
Contributor Author

leijurv commented Jan 17, 2020

can one of you guys that's merging all my PRs from literally two years ago glance at this? this is a legitimate bug.

@leijurv
Copy link
Contributor Author

leijurv commented Jan 17, 2020

X calls Y calls LogKitten.warn. LogKitten says "Y had a warning"
X calls Y calls LogKitten.logMessage(warn). LogKitten says "X had a warning"

this is a giant foot-gun and could cause very hard to debug incorrect logging. there is one example currently. LogKitten.logMessage REALLY SHOULD be a private method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants