😭 Knex JS breaks logger

# [ $davids.sh ] · message #102

😭** Knex JS breaks logger**

Knex is one of the most well-known libraries for working with SQL in NodeJS. And as expected, libraries of this kind can accept an external logger as input.

BUT instead of simply calling the logger's functions, it overrides all its methods in its own class (photo 1)...

Have you guessed what the problem might be? Answer in 3... 2... 1...

And the problem is that if your logger uses "this" without binding it to an instance (which is not mandatory), then its "this" will now refer to Knex's internal logger class...

In my recent projects, I often use the Pino logger, and inside its logic, "this" is used without binding, which leads to a crash of the application.

To make everything work, I have to use a nasty hack (photo 2).

What does this teach us?

"This" is a very sneaky thing, and of course, I want to say "just stop using class notation" (because I haven't used it for a long time), but in reality, I ask you to at least not override methods of another (especially uncontrollable) object.

Powerful leveling up to you 💪

• @ 4ront · # 162

  This rule about reassigning methods has long been carved in stone, but people stubbornly keep stepping on the same rake 😄

• @ Bogdan Node.js RentalClub · # 164

  By the way, I think I already told you about that.

• @ 🦾 IT-Dressing room 💪 · # 165

  If I remembered all the JavaScript issues we've discussed, I'd probably fall into depression...

  Though... wait a minute...

• @ Bogdan Node.js RentalClub · # 166

  Do you remember axios?

• @ 🦾 IT-Dressing room 💪 · # 167

  I’m actively working with a therapist to let go...

• @ Bogdan Node.js RentalClub · # 168

  Here's another issue with axios - turns out the maintainers clearly can't keep up with fixes in their library (and test coverage). I'm considering switching to my own client based on pure Node.

  So the problem is that timeouts stopped working correctly after replacing the agent with a pure Node one (remember we're replacing the agent because we're adding timing metrics to it). The funny thing is that theoretically it should use Node's http client by default, which you can see in the library's repo https://github.com/axios/axios/blob/199c8aab64c45532389f3e56a913017a54803dfe/lib/adapters/http.js#L189 and the imported libraries at the top. That's why I found this behavior-breaking issue particularly strange.

  After reviewing axios code in the project repo again, I noticed something I'd overlooked before - specifically the config.maxRedirects parameter. When >0, it uses http(s)Follow. Note that by default config.maxRedirects = 5, so we always end up using this agent. I used to think it was just a minor fix for the http library, but I was wrong.

  Timeout functionality isn't properly implemented in axios. It only works in the follow-redirects library, and no one bothered to test the Node client properly. The maintainers didn't even check what happens if you set maxRedirects = 0. Meanwhile, the people who wrote the follow-redirects library (which was only supposed to handle redirect following) also improved timeout logic (thereby changing it), and axios developers unknowingly relied on this feature...

  Fixed timeout logic: https://github.com/follow-redirects/follow-redirects/blob/main/index.js#L148

• @ Bogdan Node.js RentalClub · # 169

  old copypasta from the work chat

• @ 🦾 IT-Dressing room 💪 · # 172

  The quality is really crappy, I can't see anything.