getsentry / craft

The universal Sentry release CLI 🚀
MIT License
133 stars 15 forks source link

feat: Handle existing NPM packages #476

Closed mydea closed 1 year ago

mydea commented 1 year ago

When publishing a JS monorepo, if something goes wrong in the publish process, it can happen that only some packages are released, but others aren't. So you can e.g. have a state for the monorepo packages:

Now there is no way to fix the 1.0.2 release, because any rerun of this will fail when trying to publish package-1 because 1.0.2 already exists for it. See for example: https://github.com/getsentry/publish/actions/runs/5584823217/jobs/10206737755

This PR adds some special casing for this specific error, in which case we simply ignore and skip this. I don't have a great way of testing this directly, if there are any ideas let me know.

BYK commented 1 year ago

I really like the idea behind the PR however I think we should add more guardrails:

  1. This behavior is only okay on a monorepo release with a retry so the behavior should be limited to that. We may consider adding a CLI flag "--retry" to indicate this is a retry. We should also check of this is part of multi-release (not necessarily a monorepo).
  2. Instead of checking for an error string, it would be safer to check for a specific error code as the string can change without notice.
BYK commented 1 year ago

Also related: https://github.com/getsentry/craft/issues/405

mydea commented 1 year ago

Yeah, I'm also not too happy about the string matching, but the error I see is:

npm: npm ERR! code E403
  npm: npm ERR! 403 403 Forbidden - PUT https://registry.npmjs.org/@sentry-internal%2feslint-plugin-sdk - You cannot publish over the previously published versions: 7.59.1.
  npm: npm ERR! 403 In most cases, you or one of your dependencies are requesting
  npm: npm ERR! 403 a package version that is forbidden by your security policy.

So not too sure how to better detect this, because just checking 403 is probably also not a good idea 😅

Any pointers on how to detect a multi-release? 🤔 but maybe it's good enough if we simply add a new options to be passed to opt-into this behavior?

retry is maybe not an ideal name for an option, as it may be ambiguous (could also mean retry to publish x times). What about skipWhenPackageVersionExists or something like this?

getsantry[bot] commented 1 year ago

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀