drop support for obsolete `hapi` package
To be clear, I'm talking about hapi, and not about dropping @hapi/hapi.
Starting with v17.9.0 and v18.2.0 the name changed from 'hapi' to '@hapi/hapi'.
hapi (the old one) is deprecated, obsolete, unmaintained and has known major security issues. IIUC it is coming up on two years being so. https://github.com/hapijs/hapi/issues/4114 provides some details. Anything before @hapi/hapi@20 is in this "deprecated, ..." group.
% npm ci
npm WARN deprecated [email protected]: This version contains severe security issues and defects and should not be used! Please upgrade to the latest version of @hapi/hapi or consider a commercial license (https://github.com/hapijs/hapi/issues/4114)
...
There are maintenance and testing costs to continuing to support it. The monstrous .tav.yml block hints at some of the complexity: https://github.com/elastic/apm-agent-nodejs/blob/a289d4428c2c1ac3a57b0767794cb28e928c1da4/.tav.yml#L408-L464
Also, note there are lingering minor issues:
- https://github.com/npm/cli/issues/2267 is an old unfixed issue with
npm installand older versions of hapi (versions that we do currently install and test in our TAV tests) - The old 'hapi' packages used npm-shrinkwrap, which results in "extraneous" package entries in package-lock.json for npm versions before v8.6 (the default in node v16). So 'hapi' will be a contributor to noise in our package-lock updates.
open questions
- Do we drop it now, or do we wait for a major version bump of the agent?
- which results in "extraneous" package entries in package-lock.json for npm versions before v8.6
I'm wrong about this being a "npm v8.6" difference. Using npm 8.10.0 locally I sometimes get the "extraneous" entries and sometimes not... depending on the exact npm install ... command used. Sigh.
Dependabot package-lock.json updates remove “extraneous” entries. Removing those entries results in npm install (with node v14’s npm v6) failing to install the ‘hapi’ package’s deps and tests fail. I think this is now the main motivator for dropping hapi. In a trade-off between testing old/obsolete 'hapi' vs. using dependabot for security and dep updates, I'll take dependabot.
In the team meeting, @estolfo suggested dropping testing of the obsolete "hapi" package for now, and defer dropping actual support for it until the next major. I think that sounds good. I will work on a PR to try that. If that works, I'll put this issue on the "next-major" milestone.
And just make sure you document that the obsolete hapi package support is untested and deprecated so that users know they are using its instrumentation it at their own risk.
And just make sure you document that the obsolete hapi package support is untested and deprecated so that users know they are using its instrumentation it at their own risk.
Yup, that was done both in the changelog (https://github.com/elastic/apm-agent-nodejs/pull/2698/files#diff-cb0601935fcaac3fea29b215282428b964b8b6e1bd5a6b8c68e84d651633c9e3) and supported-technologies doc page (https://github.com/elastic/apm-agent-nodejs/pull/2698/files#diff-d2a92236665ca681fc14a4b85cb60e74bf3d25b42328ca2390fca53156100d25) of #2698
Resolved by https://github.com/elastic/apm-agent-nodejs/pull/3541 on the "dev/4.x" branch, which will be the 4.x active branch soonish.