feat(solidstart)!: Default to `--import` setup and add `autoInjectServerSentry` by s1gr1d · Pull Request #14862 · getsentry/sentry-javascript

s1gr1d · 2024-12-30T14:20:29Z

This PR adds a withSentry wrapper for SolidStart's config to build and place instrument.server.ts alongside the server build output so that it doesn't have to be placed in /public anymore to be discoverable.

The setup is changed to be aligned with Nuxt.
First, the instrument.server.ts file is added to the build output (the sentry release injection file needs to be copied as well - this is not ideal at the moment as there could be other imports as well, but it's okay for now)

Then, there are two options to set up the SDK:

Users provide an --import CLI flag to their start command like this:
node --import ./.output/server/instrument.server.mjs .output/server/index.mjs
Users can add autoInjectServerSentry: 'top-level-import' and the Sentry config will be imported at the top of the server entry

// app.config.ts
import { defineConfig } from '@solidjs/start/config';
import { withSentry } from '@sentry/solidstart';

export default defineConfig(withSentry(
    { /* ... */ },
    { 
      autoInjectServerSentry: 'top-level-import' // optional
    })
 );

builds on top of the idea in this PR: #13784

codecov · 2024-12-30T14:41:51Z

❌ 3 Tests Failed:

Tests completed	Failed	Passed	Skipped
254	3	251	8

View the top 3 failed tests by shortest run time

errors.server.test.ts server-side errors › captures server action error

Stack Traces | 30s run time

errors.server.test.ts:5:3 captures server action error

performance.server.test.ts sends a server action transaction on pageload

Stack Traces | 30s run time

performance.server.test.ts:9:1 sends a server action transaction on pageload

performance.server.test.ts sends a server action transaction on client navigation

Stack Traces | 30s run time

performance.server.test.ts:32:1 sends a server action transaction on client navigation

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

CHANGELOG.md

andreiborza · 2025-01-07T09:29:40Z

dev-packages/e2e-tests/test-applications/solidstart/tests/errorboundary.test.ts

+  // The first page load causes a hydration error on the dev server sometimes - a reload works around this
+  await page.reload();


m: Is this an issue again? We aren't using the dev server right?

packages/solidstart/src/config/addInstrumentation.ts

andreiborza · 2025-01-07T09:52:22Z

packages/solidstart/src/vite/sentrySolidStartVite.ts

+  // TODO: Ensure this file is source mapped too.
+  // Placing this after the sentry vite plugin means this
+  // file won't get a sourcemap and won't have a debug id injected.
+  // Because the file is just copied over to the output server
+  // directory the release injection file from sentry vite plugin
+  // wouldn't resolve correctly otherwise.
+  sentryPlugins.push(makeBuildInstrumentationFilePlugin(options));


q: Is this still relevant now that we copy over the release injection files? We probably want to have debug id injections now.

andreiborza · 2025-01-07T09:57:00Z

dev-packages/e2e-tests/test-applications/solidstart/tests/errorboundary.test.ts

+  // The first page load causes a hydration error on the dev server sometimes - a reload works around this
+  await page.reload();


l: Is this still relevant? We aren't using the dev mode anymore right?

andreiborza · 2025-01-07T10:01:34Z

packages/solidstart/src/vite/sentrySolidStartVite.ts

+  // TODO: Ensure this file is source mapped too.
+  // Placing this after the sentry vite plugin means this
+  // file won't get a sourcemap and won't have a debug id injected.
+  // Because the file is just copied over to the output server
+  // directory the release injection file from sentry vite plugin
+  // wouldn't resolve correctly otherwise.
+  sentryPlugins.push(makeBuildInstrumentationFilePlugin(options));


m: Now that we copy over release injection files, we should ensure that the instrumentation file is also properly injected with debug ids right?

…instrumentation files

…verSentry`

andreiborza

LGTM

…import` (#14863) ⚠️ THIS PR IS BASED ON #14862 Adds the option to dynamically import the server config file. ```typescript // app.config.ts import { defineConfig } from '@solidjs/start/config'; import { sentrySolidStartVite, withSentry } from '@sentry/solidstart'; export default defineConfig(withSentry( { /* ... */ }, { autoInjectServerSentry: 'experimental_dynamic-import' }) ); ``` --------- Co-authored-by: Andrei Borza <andrei.borza@sentry.io>

…erSentry` (#14862) This PR adds a `withSentry` wrapper for SolidStart's config to build and place `instrument.server.ts` alongside the server build output so that it doesn't have to be placed in `/public` anymore to be discoverable. The setup is changed to be aligned with Nuxt. First, the `instrument.server.ts` file is added to the build output (the sentry release injection file needs to be copied as well - this is not ideal at the moment as there **could** be other imports as well, but it's okay for now) Then, there are two options to set up the SDK: 1. Users provide an `--import` CLI flag to their start command like this: ```node --import ./.output/server/instrument.server.mjs .output/server/index.mjs``` 2. Users can add `autoInjectServerSentry: 'top-level-import'` and the Sentry config will be imported at the top of the server entry ```typescript // app.config.ts import { defineConfig } from '@solidjs/start/config'; import { withSentry } from '@sentry/solidstart'; export default defineConfig(withSentry( { /* ... */ }, { autoInjectServerSentry: 'top-level-import' // optional }) ); ``` --- builds on top of the idea in this PR: #13784 --------- Co-authored-by: Andrei Borza <andrei.borza@sentry.io>

…import` (#14863) ⚠️ THIS PR IS BASED ON #14862 Adds the option to dynamically import the server config file. ```typescript // app.config.ts import { defineConfig } from '@solidjs/start/config'; import { sentrySolidStartVite, withSentry } from '@sentry/solidstart'; export default defineConfig(withSentry( { /* ... */ }, { autoInjectServerSentry: 'experimental_dynamic-import' }) ); ``` --------- Co-authored-by: Andrei Borza <andrei.borza@sentry.io> (cherry picked from commit 38ff6eb)

…erSentry` (#14862) This PR adds a `withSentry` wrapper for SolidStart's config to build and place `instrument.server.ts` alongside the server build output so that it doesn't have to be placed in `/public` anymore to be discoverable. The setup is changed to be aligned with Nuxt. First, the `instrument.server.ts` file is added to the build output (the sentry release injection file needs to be copied as well - this is not ideal at the moment as there **could** be other imports as well, but it's okay for now) Then, there are two options to set up the SDK: 1. Users provide an `--import` CLI flag to their start command like this: ```node --import ./.output/server/instrument.server.mjs .output/server/index.mjs``` 2. Users can add `autoInjectServerSentry: 'top-level-import'` and the Sentry config will be imported at the top of the server entry ```typescript // app.config.ts import { defineConfig } from '@solidjs/start/config'; import { withSentry } from '@sentry/solidstart'; export default defineConfig(withSentry( { /* ... */ }, { autoInjectServerSentry: 'top-level-import' // optional }) ); ``` --- builds on top of the idea in this PR: #13784 --------- Co-authored-by: Andrei Borza <andrei.borza@sentry.io>

…import` (#14863) ⚠️ THIS PR IS BASED ON #14862 Adds the option to dynamically import the server config file. ```typescript // app.config.ts import { defineConfig } from '@solidjs/start/config'; import { sentrySolidStartVite, withSentry } from '@sentry/solidstart'; export default defineConfig(withSentry( { /* ... */ }, { autoInjectServerSentry: 'experimental_dynamic-import' }) ); ``` --------- Co-authored-by: Andrei Borza <andrei.borza@sentry.io> (cherry picked from commit 38ff6eb)

…15135) Backport of #14862 and #14863 --------- Co-authored-by: Andrei Borza <andrei.borza@sentry.io>

s1gr1d changed the title ~~Ab/solidstart withsentry~~ feat(solidstart)!: Default to --import setup and add autoInjectServerSentry Dec 30, 2024

s1gr1d changed the title ~~feat(solidstart)!: Default to --import setup and add autoInjectServerSentry~~ feat(solidstart)!: Default to --import setup and add autoInjectServerSentry Dec 30, 2024

s1gr1d requested a review from andreiborza December 30, 2024 14:21

s1gr1d force-pushed the ab/solidstart-withsentry branch from e7b59a0 to b402fe1 Compare December 30, 2024 14:22

s1gr1d mentioned this pull request Dec 30, 2024

feat(solidstart): Add autoInjectServerSentry: 'experimental_dynamic-import #14863

Merged

andreiborza reviewed Jan 7, 2025

View reviewed changes

andreiborza and others added 17 commits January 8, 2025 10:40

feat(solidstart): Add withSentry config wrapper to enable building …

9c033b5

…instrumentation files

Fix typing, unify vite plugin options

ae5efcd

Add JSDoc to withSentry

4764548

Add unit tests for config helpers

1677499

Change ordering of plugins

25d191c

Update README

9688b97

Document optional instrumentation file path option

9123819

Add sentrySolidStartVite plugin when using withSentry

fd9678e

Update README

c4b50dd

Add reload workoarund for dev server hydration error :(

99e790a

Rewrite plugin to add auto-instrument option

d2223fa

feat(solidstart)!: Default to --import setup and add `autoInjectSer…

9c1d06d

…verSentry`

fix tests

9c8ba4b

add changelog entry

61cc300

review suggestions

4631cf3

move plugin

3d8bb0b

review suggestion

76cc18b

s1gr1d force-pushed the ab/solidstart-withsentry branch from 0d73fd9 to 60dacf6 Compare January 8, 2025 09:40

s1gr1d requested a review from andreiborza January 8, 2025 09:44

fix tests

333d210

s1gr1d force-pushed the ab/solidstart-withsentry branch from 60dacf6 to 333d210 Compare January 8, 2025 09:46

andreiborza approved these changes Jan 8, 2025

View reviewed changes

s1gr1d merged commit 74e2982 into develop Jan 8, 2025

s1gr1d deleted the ab/solidstart-withsentry branch January 8, 2025 11:10

s1gr1d mentioned this pull request Jan 8, 2025

[SolidStart] Update setup docs based on new withSentry method getsentry/sentry-docs#12279

Closed

s1gr1d mentioned this pull request Jan 14, 2025

docs(solidstart): Add docs for installation methods getsentry/sentry-docs#12335

Merged

3 tasks

s1gr1d mentioned this pull request Jan 22, 2025

feat(v8/solidstart): Add withSentry wrapper for SolidStart config #15135

Merged

s1gr1d added a commit that referenced this pull request Jan 23, 2025

feat(v8/solidstart): Add withSentry wrapper for SolidStart config (#…

a76f243

…15135) Backport of #14862 and #14863 --------- Co-authored-by: Andrei Borza <andrei.borza@sentry.io>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(solidstart)!: Default to `--import` setup and add `autoInjectServerSentry`#14862

feat(solidstart)!: Default to `--import` setup and add `autoInjectServerSentry`#14862
s1gr1d merged 18 commits intodevelopfrom
ab/solidstart-withsentry

s1gr1d commented Dec 30, 2024 •

edited

Loading

Uh oh!

codecov bot commented Dec 30, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

andreiborza Jan 7, 2025

Uh oh!

Uh oh!

andreiborza Jan 7, 2025

Uh oh!

andreiborza Jan 7, 2025

Uh oh!

andreiborza Jan 7, 2025

Uh oh!

andreiborza left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// The first page load causes a hydration error on the dev server sometimes - a reload works around this
		await page.reload();

Uh oh!

Conversation

s1gr1d commented Dec 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 3 Tests Failed:

Uh oh!

Uh oh!

Uh oh!

andreiborza Jan 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andreiborza Jan 7, 2025

Choose a reason for hiding this comment

Uh oh!

andreiborza Jan 7, 2025

Choose a reason for hiding this comment

Uh oh!

andreiborza Jan 7, 2025

Choose a reason for hiding this comment

Uh oh!

andreiborza left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

s1gr1d commented Dec 30, 2024 •

edited

Loading

codecov bot commented Dec 30, 2024 •

edited

Loading