Deprecate and archive the good modules suite

hapijs / good

hapi process monitoring

Other

525 stars 161 forks source link

Deprecate and archive the good modules suite #641

Open spridev opened 3 years ago

spridev commented 3 years ago

Support plan

is this issue currently blocking your project? (yes/no): no
is this issue affecting a production system? (yes/no): no

Context

node version: 14.15.0
module version: 9.0.1
environment (e.g. node, browser, native): node
used with (e.g. hapi application, another framework, standalone, ...): -
any other relevant information: -

What problem are you trying to solve?

As mentioned in the README, this package has been deprecated on December 31st 2020.

Do you have a new or modified API suggestion to solve the problem?

I would suggest:

Marking @hapi/good as deprecated on npm.
Archiving this repository.
(Removing the README deprecation notice which will end up being redundant).

The same applies to the two following modules:

It seems like the documentation for those modules is already gone from hapi.dev.

Finally, @hapi/log could be mentioned as an alternative (?).

Nargonath commented 3 years ago

@hapi/log is not ready for usage right now. I believe we don't want to really deprecate good until we have something else to offer. Thanks for the reminder though.

TinaHeiligers commented 2 years ago

We're working with a fork of good and have recently started seeing memory leaks that appear to be related to the stream not draining. Understandably, good has been deprecated and we've since moved on to an in-house, custom logging solution. However, I'm curious to know if anyone else has encountered similar memory-consumption issues?

Nargonath commented 2 years ago

@TinaHeiligers I believe there were changes in how streams were implemented in node@18 which can be a reason for cleanup code not to be called. Depending on the node version you're using I'd scan node's changelogs to know if you're impacted by some of these changes.

kanongil commented 2 years ago

Streams issues impacting older streams implementations, that have issues with never versions of node, can sometimes be caused by the introduction and defaults of the emitClose and autoDestroy stream constructor options. Updating the old code, so the superclass constructor is called with the options explicitly set to false, can sometimes fix this.

TinaHeiligers commented 2 years ago

@Nargonath ATM, the code in question is running on node 16.14.2, so I don't think the issues we're seeing stem from that. However, you mention an important point, and we plan on being vigilant with the next node upgrade.

@kanongil Thanks for the tip! I'll bring it to the team.