| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
size-limit report 📦
|
Sorry, something went wrong.
|
This pull request 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 apply the label PR: no-auto-close I will leave it alone ... forever! |
Sorry, something went wrong.
|
bugbot run |
Sorry, something went wrong.
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit af132f5. Configure here.
Sorry, something went wrong.
…nfig SvelteKit 3.0.0-next.8 promoted the native server-tracing config out of `experimental` (`kit.experimental.tracing.server` -> `kit.tracing.server`). Read both locations when detecting native tracing so the SDK works across the supported peer range (SvelteKit 2.31+ and early Kit 3 prereleases keep it under `experimental`). Applies to detection from both the Vite plugin `api.options` and `svelte.config.js`, and covers both shapes in the unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| // Prefer the environment of the current build (`this.environment.name === 'ssr'`) and fall back to | ||
| // `isServerBuild` for older Vite versions that don't expose environments. | ||
| const environmentName = (this as { environment?: { name?: string } }).environment?.name; | ||
| const isServerEnvironment = environmentName != null ? environmentName === 'ssr' : !!isServerBuild; |
There was a problem hiding this comment.
Bug: The code hardcodes the server environment name as 'ssr'. This will fail for users who customize the environment name in Vite, leading to incorrect server-side instrumentation.
Severity: LOW
Avoid hardcoding the environment name 'ssr'. Instead, determine if the environment is server-side by a more robust method. For example, a more flexible check could be to consider any environment that is not explicitly the 'client' environment as a server-side one, which would accommodate custom names.
Prompt for AI AgentReview 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: packages/sveltekit/src/vite/autoInstrument.ts#L82 Potential issue: The logic at `autoInstrument.ts` assumes the Vite server environment is always named `'ssr'`. However, Vite's Environment API allows users to customize this name (e.g., to `'server'`). If a custom name is used and `onlyInstrumentClient` is `true`, the `isServerEnvironment` check will incorrectly evaluate to `false`. This causes server-side universal load functions to be instrumented when they should be skipped, resulting in incorrect stack frames for server-side errors in production for users with this specific configuration.
Did we get this right? 👍 / 👎 to inform future reviews.
Sorry, something went wrong.
There was a problem hiding this comment.
This is set by SvelteKit, right?
Sorry, something went wrong.
| // Prefer the environment of the current build (`this.environment.name === 'ssr'`) and fall back to | ||
| // `isServerBuild` for older Vite versions that don't expose environments. | ||
| const environmentName = (this as { environment?: { name?: string } }).environment?.name; | ||
| const isServerEnvironment = environmentName != null ? environmentName === 'ssr' : !!isServerBuild; |
There was a problem hiding this comment.
This is set by SvelteKit, right?
Sorry, something went wrong.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| Back | FazBrowse Home | New Git URL |
Gets the environment using this.environment.name and falls back to config.build.ssr for older Vite versions.
Un-skips the last remaining server-side error test in the Kit 3 e2e app.
ref #21502