Skip to content
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

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

Open
akwodkiewicz opened this issue Jul 2, 2024 · 3 comments
Open

Comments

@akwodkiewicz
Copy link

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 [email protected] 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
Copy link
Owner

mentos1386 commented Jul 2, 2024

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
Copy link
Author

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
Copy link

lkreimann commented Sep 24, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants