ci: Do not auto-publish the latest version to the CDN#311
Conversation
Before we were auto-publishing the `latest` tag to the CDN. This would be picked up by react users who have the `useSentryToolbar()` hook installed, and anyone who copy+pasted the CDN install instruction. I liked this before when it was early days, but it has the downside that if we publish a bad build everyone is affected. So its time to turn that off, going forward we can manually publish `latest` as a safety measure.
| - path: /sentry-toolbar/{{version}}/ | ||
| metadata: | ||
| cacheControl: 'public, max-age=60' | ||
| # Latest CDN Bundle Target | ||
| - name: gcs | ||
| id: 'browser-cdn-bundles' | ||
| includeNames: /.*\.js.*$/ | ||
| bucket: sentry-js-sdk | ||
| paths: | ||
| - path: /sentry-toolbar/latest/ | ||
| metadata: | ||
| cacheControl: 'public, max-age=60' | ||
| # NPM package target | ||
| - name: npm | ||
| id: '@sentry/toolbar' |
There was a problem hiding this comment.
Bug: The useSentryToolbar hook defaults to loading from the /latest/ CDN path, but this PR removes the publishing of that path, causing a 404 error.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
This pull request removes the automatic publishing of the /sentry-toolbar/latest/ CDN target in .craft.yml. However, the useSentryToolbar hook still defaults to this path. The versionToCdn function is called with a default version of 'latest' when no cdn or version is provided. Since the documented usage pattern encourages omitting these parameters, users following the documentation will experience a 404 error when the toolbar attempts to load, breaking the feature.
💡 Suggested Fix
Update the useSentryToolbar hook to no longer default to the 'latest' version. Either require a specific version or CDN URL to be passed, or change the default to a stable, published version. Additionally, update all documentation that references the /latest/ path.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: .craft.yml#L15-L20
Potential issue: This pull request removes the automatic publishing of the
`/sentry-toolbar/latest/` CDN target in `.craft.yml`. However, the `useSentryToolbar`
hook still defaults to this path. The `versionToCdn` function is called with a default
`version` of `'latest'` when no `cdn` or `version` is provided. Since the documented
usage pattern encourages omitting these parameters, users following the documentation
will experience a 404 error when the toolbar attempts to load, breaking the feature.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8114171
Before we were auto-publishing the
latesttag to the CDN. This would be picked up by react users who have theuseSentryToolbar()hook installed, and anyone who copy+pasted the CDN install instruction. I liked this before when it was early days, but it has the downside that if we publish a bad build everyone is affected.So its time to turn that off, going forward we can manually publish
latestas a safety measure.