rust-lang / crates.io

The Rust package registry
https://crates.io
Apache License 2.0
2.86k stars 585 forks source link

TypeError: Cannot read property 'prerelease' of null #3294

Closed sentry-io[bot] closed 3 years ago

sentry-io[bot] commented 3 years ago

Sentry Issue: CRATES-IO-FRONTEND-P0

TypeError: Cannot read property 'prerelease' of null
  at isPrerelease (cargo/models/version.js:155:1)
  at obj (cargo/models/version.js:166:1)
  at _getProp (@ember/-internals/metal/index.js:1375:1)
  at compute (@glimmer/reference.js:213:1)
  at callback (@glimmer/reference.js:161:1)
...
(45 additional frame(s) were not displayed)
Turbo87 commented 3 years ago

I guess this is somewhat related to #3293

Turbo87 commented 3 years ago

public sentry link for this issue is: https://sentry.io/share/issue/8c078bc680be402eb2dfd7ef90e85000/ :)

plaets commented 3 years ago

Any URLs this could be reproduced at?

Turbo87 commented 3 years ago

yes, https://crates.io/crates/test-max-version-example-crate/versions

Turbo87 commented 3 years ago

https://crates.io/crates/hxgm30-client/versions too

plaets commented 3 years ago

I can reproduce this on 49e70576e88201bf542aff351c39e17d25eff33a (current prod according to whatsdeployed.io) but when I swtich to the latest commit, I get this instead

Could not find module `ember-tooltips/components/ember-tooltip` imported from `cargo/components/ember-tooltip`

That recursive duplication of the interface does not occur at all, but I'm assuming it's some sort of recovery mechanism I don't know about kicking in.

Turbo87 commented 3 years ago

you might have to yarn install and restart ember serve when switching to the current master

plaets commented 3 years ago

I'm not sure if anyone is working on this right now, if so, feel free to just ignore this comment.

This happens because semverParse returns null if a semver string is invalid. semverParse supports an option for parsing "not-quite-valid" semver strings ({'loose': true} as the second argument) and it definitely should be used in get semver. This works well for 0.4.0-alpha.01, but does not work at all for test-max-version-example-crate (for anything bigger than Number.MAX_SAFE_INTEGER), so I guess we still need some sort of a fallback for this one case, at least unless we fork node-semver.

I think that adding a check if this.semver is null in isHighestOfReleaseTrack and isPrerelease from models/version.js should be enough, I guess these two methods can just return false (or null, as in "not sure"?) if that's the case. Another check will be needed for sameTrackVersions, is just ignoring invalid versions there ok?

Things are more complicated with releaseTrack as its used in components/version-list/row.js and components/version-list/row.hbs. Is adding a new release-track tooltip message and icon a good enough solution? I couldn't find any good icons for this in assets.

Turbo87 commented 3 years ago

using loose mode is definitely a good idea, yeah.

I think that adding a check if this.semver is null in isHighestOfReleaseTrack and isPrerelease from models/version.js should be enough, I guess these two methods can just return false

agreed 👍

Is adding a new release-track tooltip message and icon a good enough solution? I couldn't find any good icons for this in assets.

yeah, I guess we could return null from releaseTrack, display ? in the release track indicator and put in a tooltip saying something like "failed to parse version XYZ".