mentos1386 / nest-raven

Sentry Module for Nest.js Framework
MIT License
249 stars 26 forks source link

Declare particular versions of @sentry/node in peerDependencies #695

Open akwodkiewicz opened 4 months ago

akwodkiewicz commented 4 months ago

Currently the package's manifest says that it's compatible with any version of @sentry/node.

I think you could improve robustness of the package by defining the @sentry/node version as 8. Or maybe even more versions, like >= 7.60 < 9, since I see in your package-lock.json that you're using 7.60 locally when developing the package (so I bet it works with 7.60). It might turn out in the future that there will be a particular minor version of 8 that is incompatible, but it's way better to at least specify the major than *. Also, I don't expect Sentry to do a breaking change in a minor version.

Why the request? I've just experienced an issue with a Nest project that was using nest-raven@10.0.1 and upgraded its @sentry/node to 8.12 without upgrading nest-raven. Unfortunately nest-raven in version 10.0.1 turned out not to be compatible with @sentry/node@8, because they deprecated the Handlers.parseRequest you were using there (and you've dropped it in the newest version, I've seen the #650 PR).

If the old 10.0.1 version declared its peer dependency on @sentry/node as 7, devs would know that nest-raven was incompatible with @sentry/node@8 (package manager would warn me about that) and I'd review the incompatible dependencies and searched for compatible versions.

I'm not suggesting to retroactively fix the manifest for the old versions (is that even possible?), but if you could start defining the peer dependency for @sentry/node for the next nest-raven versions more precisely, somebody might avoid a broken HTTP interceptor (returning 500 errors) in their project. 🙌

mentos1386 commented 4 months ago

Hey @akwodkiewicz, thanks for creating this issue. We should definitely constrain versions of all peerDependencies in our package.json.

Retroactively is impossible, best we could do is to mark those old releases as "deprecated" to warn people or something. But i doubt it's worth it?

Another thing we could do to improve robustness of the module is to use something like this test-all-versions tool, which we could use to run test suite against multiple versions of @sentry/node.

akwodkiewicz commented 4 months ago

There might be some other consequences of deprecating old versions (maybe old versions are the only ones working with some older versions of Nest, and you'd be forcing people to upgrade Nest this way).

Anyway, it's great to hear some ideas on how to extend the solution!

PS We'll be upgrading nest-raven to the latest version, so I just wanted to ask -- do you know if it works with @sentry/node@8?😄

lkreimann commented 1 month ago

We tried using @sentry/node v8.31.0 and nest-raven v10.1.0 and it didn't work. We still hand an error with "Handlers"

EDIT: Ah no sorry, the error was the following: image We still used Handlers in our main.ts