-
Notifications
You must be signed in to change notification settings - Fork 21
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
why patch standard Logger -- conflicting with other gems patch to infinite loop #60
Comments
HI @jrochkind this overloading was done to ensure backwards compatibility (https://github.com/rudionrails/yell/blob/master/spec/compatibility/level_spec.rb). In case your request is related to a problem with Yell, it would be best to remove the library from your project. Try your code again and if the problem still exists, please provide the stacktrace for further investigation. |
Hi, thanks for the response. I'm not sure what you mean by "backwards compatibility" -- apparently you are trying to make sure that ordinary ruby Logger instances can act in some way as if they were Yell instances? That isn't normally what's meant by "backwards compatibility". I wonder if you would consider a future yell release that does not do this patching, or makes it optional opt-in in some way? It would probably have to be a major version release, as it could break some apps, for sure. I am having a lot of trouble getting to the bottom of the problem in my project -- it exhibits in a very odd way, I haven't been been able to demonstrate it in a simplified reproduction outside of my very complex project. In my project, yell is not a direct dependency, but a transitive dependency -- a dependency of a dependency. And removing it would make the direct dependency stop working entirely. So it's not so easy to to remove. One thing I have discovered is that putting an explicit I'll put the stack trace below, it's pretty impenetrable. But it's occuring from a portion of my app I didn't think was using yell at all, the only reason yell is involved at all is because of the patching, such that yell is involved in any use of any logger anywhere. :(
|
It looks like it could be some kind of a competition between something else also trying to override the default |
OK, yeah, so this is interesting. Both yell and honeybadger are trying to add custom behavior on to ::Logger#add, and then call original implementation. yell does so using the old honeybadger does so with the newer (ruby 2.0) Module prepend, prepending this module In my app exhibiting the problem, somehow when yell code calls And then when honeybadger calls super on line 17, it ends up calling the yell implementation. Both were trying to call 'original' implementation, but neither ever gets there, they just call themselves in infinite loop. I am not sure why; I can't so far reproduce this outside of my particular complex app. It is apparently some kind of race condition in exactly how they wind up loaded in my app. Although I'm still trying to reproduce.
|
instead of older style alias_method chain juggling. We hope this has fewer poor interactions with other things patching Logger as in rudionrails#60. And it's just a more modern cleaner style. Could be a first step to making this stdlib Logger patching optional, but does not require that.
I can't manage to reproduce this in an isolated app outside of my own complicated app. Something in my app is resulting in a race condition in how yell's patching of ::Logger and honeybadger's patching of ::Logger are loaded, so they inconsistently refer to each other and cause an infinite loop. I think I'm going to give up on reproducing, just banging my head against the wall. But I think it's quite likely that #61 would solve it, and it seems to me to be a code cleanup either way. Curious your thoughts, thanks! |
to avoid some kind of infinite loop, possibly involving Rails 6 zeitwerk auto-loader. Something is really wrong, this ought not to be necessary, somewhat regretting traject using yell. But for now, this simple thing seems to resolve problem. rudionrails/yell#60 rudionrails/yell#61
I am trying to debug something really odd that seems to involve Yell and the Rails6 zeitwerk auto-loader. If/when I get to the bottom of it, it yell is implicated, I will post a different report.
However, in trying to debug, I ran into something I didn't expect. In an area of code that I was not expecting to be using yell at all, but just using a standard ruby and/or Rails
logger
, I was still finding Yell in my stack trace.Then I discovered yell is patching the ruby stdlib
Logger
class with custom behavior: https://github.com/rudionrails/yell/blob/master/lib/core_ext/logger.rbI think in 2020 this is considered pretty bad behavior. It can cause unexpected problems. It is at least plausibly responsible for the really odd and hard to figure out problem I am currently banging my head against.
Is it really necessary? Why can't Yell simply provide the custom
Yell
logger, and leave stdlib logger untouched?The text was updated successfully, but these errors were encountered: