diff --git a/.craft.yml b/.craft.yml index b65e012eb5e7..66fc8fca329c 100644 --- a/.craft.yml +++ b/.craft.yml @@ -160,28 +160,34 @@ targets: # Sentry Release Registry Target - name: registry sdks: + 'npm:@sentry/angular': + onlyIfPresent: /^sentry-angular-\d.*\.tgz$/ + 'npm:@sentry/astro': + onlyIfPresent: /^sentry-astro-\d.*\.tgz$/ + 'npm:@sentry/aws-serverless': + onlyIfPresent: /^sentry-aws-serverless-\d.*\.tgz$/ 'npm:@sentry/browser': onlyIfPresent: /^sentry-browser-\d.*\.tgz$/ includeNames: /\.js$/ checksums: - algorithm: sha384 format: base64 - 'npm:@sentry/node': - onlyIfPresent: /^sentry-node-\d.*\.tgz$/ - 'npm:@sentry/react': - onlyIfPresent: /^sentry-react-\d.*\.tgz$/ - 'npm:@sentry/vue': - onlyIfPresent: /^sentry-vue-\d.*\.tgz$/ + 'npm:@sentry/bun': + onlyIfPresent: /^sentry-bun-\d.*\.tgz$/ + 'npm:@sentry/deno': + onlyIfPresent: /^sentry-deno-\d.*\.tgz$/ + 'npm:@sentry/ember': + onlyIfPresent: /^sentry-ember-\d.*\.tgz$/ 'npm:@sentry/gatsby': onlyIfPresent: /^sentry-gatsby-\d.*\.tgz$/ - 'npm:@sentry/angular': - onlyIfPresent: /^sentry-angular-\d.*\.tgz$/ - 'npm:@sentry/astro': - onlyIfPresent: /^sentry-astro-\d.*\.tgz$/ - 'npm:@sentry/wasm': - onlyIfPresent: /^sentry-wasm-\d.*\.tgz$/ + 'npm:@sentry/google-cloud-serverless': + onlyIfPresent: /^sentry-google-cloud-serverless-\d.*\.tgz$/ 'npm:@sentry/nextjs': onlyIfPresent: /^sentry-nextjs-\d.*\.tgz$/ + 'npm:@sentry/node': + onlyIfPresent: /^sentry-node-\d.*\.tgz$/ + 'npm:@sentry/react': + onlyIfPresent: /^sentry-react-\d.*\.tgz$/ 'npm:@sentry/remix': onlyIfPresent: /^sentry-remix-\d.*\.tgz$/ 'npm:@sentry/solid': @@ -190,9 +196,9 @@ targets: onlyIfPresent: /^sentry-svelte-\d.*\.tgz$/ 'npm:@sentry/sveltekit': onlyIfPresent: /^sentry-sveltekit-\d.*\.tgz$/ - 'npm:@sentry/bun': - onlyIfPresent: /^sentry-bun-\d.*\.tgz$/ 'npm:@sentry/vercel-edge': onlyIfPresent: /^sentry-vercel-edge-\d.*\.tgz$/ - 'npm:@sentry/ember': - onlyIfPresent: /^sentry-ember-\d.*\.tgz$/ + 'npm:@sentry/vue': + onlyIfPresent: /^sentry-vue-\d.*\.tgz$/ + 'npm:@sentry/wasm': + onlyIfPresent: /^sentry-wasm-\d.*\.tgz$/ diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5e100df939ad..c8505ab3a022 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1477,33 +1477,33 @@ jobs: node: 22 # macos x64 - - os: macos-11 + - os: macos-13 node: 16 arch: x64 - - os: macos-11 + - os: macos-13 node: 18 arch: x64 - - os: macos-11 + - os: macos-13 node: 20 arch: x64 - - os: macos-11 + - os: macos-13 node: 22 arch: x64 # macos arm64 - - os: macos-12 + - os: macos-13 arch: arm64 node: 16 target_platform: darwin - - os: macos-12 + - os: macos-13 arch: arm64 node: 18 target_platform: darwin - - os: macos-12 + - os: macos-13 arch: arm64 node: 20 target_platform: darwin - - os: macos-12 + - os: macos-13 arch: arm64 node: 22 target_platform: darwin diff --git a/CHANGELOG.md b/CHANGELOG.md index a54112be9a29..027b13f56901 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,28 @@ ### Important Changes +- **feat(solid): Remove need to pass router hooks to solid integration** (breaking) + +This release introduces breaking changes to the `@sentry/solid` package (which is currently out in alpha). + +We've made it easier to get started with the solid router integration by removing the need to pass **use\*** hooks +explicitly to `solidRouterBrowserTracingIntegration`. Import `solidRouterBrowserTracingIntegration` from +`@sentry/solid/solidrouter` and add it to `Sentry.init` + +```js +import * as Sentry from '@sentry/solid'; +import { solidRouterBrowserTracingIntegration, withSentryRouterRouting } from '@sentry/solid/solidrouter'; +import { Router } from '@solidjs/router'; + +Sentry.init({ + dsn: '__PUBLIC_DSN__', + integrations: [solidRouterBrowserTracingIntegration()], + tracesSampleRate: 1.0, // Capture 100% of the transactions +}); + +const SentryRouter = withSentryRouterRouting(Router); +``` + - **feat(core): Return client from init method (#12585)** `Sentry.init()` now returns a client directly, so you don't need to explicitly call `getClient()` anymore: @@ -28,10 +50,21 @@ module.exports = withSentryConfig(nextConfig, { }); ``` +- **feat(node): Allow to configure `maxSpanWaitDuration` (#12610)** + +Adds configuration option for the max. duration in seconds that the SDK will wait for parent spans to be finished before +discarding a span. The SDK will automatically clean up spans that have no finished parent after this duration. This is +necessary to prevent memory leaks in case of parent spans that are never finished or otherwise dropped/missing. However, +if you have very long-running spans in your application, a shorter duration might cause spans to be discarded too early. +In this case, you can increase this duration to a value that fits your expected data. + ### Other Changes - feat(feedback): Extra check for iPad in screenshot support (#12593) - fix(bundle): Ensure CDN bundles do not overwrite `window.Sentry` (#12580) +- fix(feedback): Inject preact from feedbackModal into feedbackScreenshot integration (#12535) +- fix(node): Re-throw errors from koa middleware (#12609) +- fix(remix): Mark `isRemixV2` as optional in exposed types. (#12614) - ref(node): Add error message to NodeFetch log (#12612) Work in this release was contributed by @n4bb12. Thank you for your contribution! diff --git a/dev-packages/e2e-tests/test-applications/node-koa/index.js b/dev-packages/e2e-tests/test-applications/node-koa/index.js index 08ddc231ffb3..ddc17f62e6f7 100644 --- a/dev-packages/e2e-tests/test-applications/node-koa/index.js +++ b/dev-packages/e2e-tests/test-applications/node-koa/index.js @@ -103,6 +103,12 @@ router1.get('/test-outgoing-http-external-disallowed', async ctx => { ctx.body = data; }); +router1.get('/test-assert/:condition', async ctx => { + ctx.body = 200; + const condition = ctx.params.condition !== 'false'; + ctx.assert(condition, 400, 'ctx.assert failed'); +}); + app1.use(router1.routes()).use(router1.allowedMethods()); app1.listen(port1); diff --git a/dev-packages/e2e-tests/test-applications/node-koa/tests/assert.test.ts b/dev-packages/e2e-tests/test-applications/node-koa/tests/assert.test.ts new file mode 100644 index 000000000000..0f9f724ef237 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/node-koa/tests/assert.test.ts @@ -0,0 +1,35 @@ +import { expect, test } from '@playwright/test'; +import { waitForError } from '@sentry-internal/test-utils'; + +test('Returns 400 from failed assert', async ({ baseURL }) => { + const errorEventPromise = waitForError('node-koa', event => { + return !event.type && event.exception?.values?.[0]?.value === 'ctx.assert failed'; + }); + + const res = await fetch(`${baseURL}/test-assert/false`); + expect(res.status).toBe(400); + + const errorEvent = await errorEventPromise; + + expect(errorEvent.exception?.values).toHaveLength(1); + expect(errorEvent.exception?.values?.[0]?.value).toBe('ctx.assert failed'); + + expect(errorEvent.request).toEqual({ + method: 'GET', + cookies: {}, + headers: expect.any(Object), + url: 'http://localhost:3030/test-assert/false', + }); + + expect(errorEvent.transaction).toEqual('GET /test-assert/:condition'); + + expect(errorEvent.contexts?.trace).toEqual({ + trace_id: expect.any(String), + span_id: expect.any(String), + }); +}); + +test('Returns 200 from successful assert', async ({ baseURL }) => { + const res = await fetch(`${baseURL}/test-assert/true`); + expect(res.status).toBe(200); +}); diff --git a/dev-packages/e2e-tests/test-applications/solid/src/index.tsx b/dev-packages/e2e-tests/test-applications/solid/src/index.tsx index b975502ef590..66773f009d1e 100644 --- a/dev-packages/e2e-tests/test-applications/solid/src/index.tsx +++ b/dev-packages/e2e-tests/test-applications/solid/src/index.tsx @@ -1,6 +1,7 @@ /* @refresh reload */ import * as Sentry from '@sentry/solid'; -import { Router, useBeforeLeave, useLocation } from '@solidjs/router'; +import { solidRouterBrowserTracingIntegration, withSentryRouterRouting } from '@sentry/solid/solidrouter'; +import { Router } from '@solidjs/router'; import { render } from 'solid-js/web'; import './index.css'; import PageRoot from './pageroot'; @@ -10,12 +11,12 @@ Sentry.init({ dsn: import.meta.env.PUBLIC_E2E_TEST_DSN, debug: true, environment: 'qa', // dynamic sampling bias to keep transactions - integrations: [Sentry.solidRouterBrowserTracingIntegration({ useBeforeLeave, useLocation })], + integrations: [solidRouterBrowserTracingIntegration()], release: 'e2e-test', tunnel: 'http://localhost:3031/', // proxy server tracesSampleRate: 1.0, }); -const SentryRouter = Sentry.withSentryRouterRouting(Router); +const SentryRouter = withSentryRouterRouting(Router); render(() => {routes}, document.getElementById('root')); diff --git a/docs/migration/feedback.md b/docs/migration/feedback.md index 6d9c189df1c2..c6f328c9ff0f 100644 --- a/docs/migration/feedback.md +++ b/docs/migration/feedback.md @@ -14,15 +14,15 @@ Below you can find a list of relevant feedback changes and issues that have been We have streamlined the interface for interacting with the Feedback widget. Below is a list of public functions that existed in 7.x and a description of how they have changed in v8. -| Method Name | Replacement | Notes | -| ------------------------------------------------------------- | -------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `Sentry.getClient()?.getIntegration(Feedback)` | `const feedback = Sentry.getFeedback()` | Get a type-safe reference to the configured feedbackIntegration instance. | -| `feedback.getWidget()` | `const widget = feedback.createWidget(); widget.appendToDom()` | The SDK no longer maintains a stack of form instances. If you call `createWidget()` a new widget will be inserted into the DOM and an `ActorComponent` returned allows you control over the lifecycle of the widget. | -| `feedback.openDialog()` | `widget.open()` | Make the form inside the widget visible. | -| `feedback.closeDialog()` | `widget.close()` | Make the form inside the widget hidden in the page. Success/Error messages will still be rendered and will hide themselves if the form was recently submitted. | -| `feedback.removeWidget()` | `widget.removeFromDom()` | Remove the form and widget instance from the page. After calling this `widget.el.parentNode` will be set to null. | -| `feedback.attachTo()` | `const unsubscribe = feedback.attachTo(myButtonElem)` | The `attachTo()` method will create an onClick event listener to your html element that calls `appendToDom()` and `open()`. It returns a callback to remove the event listener. | -| - | `const form = await feedback.createForm()` | A new method `createForm()`, used internally by `createWidget()` and `attachTo()`, returns a `Promise` so you can control showing and hiding of the feedback form directly. | +| Method Name | Replacement | Notes | +| ------------------------------------------------------------- | -------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Sentry.getClient()?.getIntegration(Feedback)` | `const feedback = Sentry.getFeedback()` | Get a type-safe reference to the configured feedbackIntegration instance. | +| `feedback.getWidget()` | `const widget = feedback.createWidget(); widget.appendToDom()` | The SDK no longer maintains a stack of form instances. If you call `createWidget()` a new widget will be inserted into the DOM and an `ActorComponent` returned allows you control over the lifecycle of the widget. | +| `feedback.openDialog()` | `widget.open()` | Make the form inside the widget visible. | +| `feedback.closeDialog()` | `widget.close()` | Make the form inside the widget hidden in the page. Success/Error messages will still be rendered and will hide themselves if the form was recently submitted. | +| `feedback.removeWidget()` | `widget.removeFromDom()` | Remove the form and widget instance from the page. After calling this `widget.el.parentNode` will be set to null. | +| `feedback.attachTo()` | `const unsubscribe = feedback.attachTo(myButtonElem)` | The `attachTo()` method will create an onClick event listener to your html element that calls `appendToDom()` and `open()`. It returns a callback to remove the event listener. | +| - | `const form = await feedback.createForm()` | A new method `createForm()`, used internally by `createWidget()` and `attachTo()`, returns a `Promise>` so you can control showing and hiding of the feedback form directly. | ### API Examples diff --git a/docs/publishing-a-release.md b/docs/publishing-a-release.md index 25c8a3a5b6f0..99025b7804a2 100644 --- a/docs/publishing-a-release.md +++ b/docs/publishing-a-release.md @@ -13,7 +13,7 @@ _These steps are only relevant to Sentry employees when preparing and publishing 4. Open a PR with the title `meta(changelog): Update changelog for VERSION` against `master` branch. 5. **Be cautious!** The PR against `master` should be merged via "Merge Commit" 6. When the PR is merged, it will automatically trigger the - [Prepare Release](https://github.com/getsentry/sentry-javascript/actions/workflows/release.yml) on master. + [Auto Prepare Release](https://github.com/getsentry/sentry-javascript/actions/workflows/auto-release.yml) on master. 7. A new issue should appear in https://github.com/getsentry/publish/issues. 8. Wait until the CI check runs have finished successfully (there is a link to them in the issue). 9. Once CI passes successfully, ask a member of the diff --git a/docs/triaging.md b/docs/triaging.md new file mode 100644 index 000000000000..cd0b1ea1aa65 --- /dev/null +++ b/docs/triaging.md @@ -0,0 +1,100 @@ +# Triaging + +The term _triage_ originally comes from medicine and describes the process of quickly examining patients who are taken +to a hospital in order to decide which ones are the most seriously ill and must be treated first. + +By _triaging issues_, we are evaluating problems that our customers are facing and providing the appropriate level of +support. The goal is to provide attention to all open issues, categorise them, and alert people when there are issues of +high severity. The goal is _not_ to fix all issues or answer all the questions coming from the open source community +immediately. + +## Bug fixing 101 + +Not every bug is equally critical or time sensitive. Some things reported as bugs aren’t even bugs. If you are unsure +whether something needs fixing, just reach out to your colleagues and get their opinion. When you do fix a bug, it +should always go hand-in-hand with adding new tests (or improving existing ones), so we can avoid any regressions in the +future. + +## Triaging workflow + +There are a few different ways to triage issues: + +1. You can look at the `#feed-web-frontend` channel in Slack. This channel will automatically receive a message every + day in the morning with issues that require triaging. +2. You can look at the triage view in the GitHub Project Board: https://github.com/orgs/getsentry/projects/31/views/29 +3. (Also for external contributors) You can filter by `Waiting for: Product Owner` label: + https://github.com/getsentry/sentry-javascript/issues?q=is%3Aopen+is%3Aissue+label%3A%22Waiting+for%3A+Product+Owner%22 + +Generally, all new issues that are opened by external users will receive the `Waiting for: Product Owner` label +initially. Whenever a contributor replies to the issue, the label will be removed automatically. If/when an external +user replies again, the label will be re-added (indicating that a response from the repo owners is expected). + +Note that issues created by contributors themselves will not get this label applied. They will also not be added to the +"Web SDK Frontend" board automatically. You'll have to add the "Web SDK Frontend" project manually to issues you create +yourself as a contributor. + +If a user replies to an issue, leading to the label being re-applied, but no response is required by a contributor, you +may also remove the label manually, which will also remove it from the triage list. + +Working through the triage queue should have the highest priority of tasks. Especially issues that are reaching the top +of the triage queue (which is indicated in the `#feed-web-frontend` channel through a remaining time to triage) should +be prioritised. **This does not mean that you need to fix the issue immediately,** but that you should investigate and +categorize the issue as soon as possible. If an issue is hard to fix, an edge case, or otherwise unclear, feel free to +reply and put the issue in backlog. You may also encourage the user to contribute a PR themselves if we are unlikely to +find time to resolve the issue ourselves anytime soon. + +### (Sentry Employees) How & when should I triage issues? + +Ideally, you can take some time every day in the morning to look over the triage queue and identify issues that you can +help triage. You will not be able to triage _every_ issue effectively, and it's OK to skip some issues if you don't know +what to do. That being said, it's important to split the triaging duty between the team members, so if you see a large +amount of issues that you cannot help with, try to find ways to help team members with their triage load in other ways. +Sometimes, this will mean taking some extra time to look into an issue. But remember, even if it takes you longer to +look into an issue than another colleague, you'll also learn stuff and you'll be more effective at triaging in the +future. + +When you start looking into an issue, you may assign the issue to yourself. This indicates to other colleagues that +somebody else is already looking into the issue. Generally speaking, the first person to assign themselves/answer in the +issue is considered the owner of this triaging issue, and other colleagues will generally not look into this issue +anymore unless prompted. Still, if you stumble upon an issue and you feel like you have something productive to add to +the conversation, feel empowered to also comment on issues owned by somebody else. Make sure to follow up on issues you +started to triage, and/or pull in other colleagues as needed. + +If a team member is out of office, make sure that issues this person started to triage continue to receive attention. + +You can and should also move issues through the project board. You can set the status to: + +- `Backlog`: May be done at some point +- `Todo`: Should be done, feel free to pick up this issue any time +- `In Progress`: This is being worked on +- `In Review`: PR is open +- `Done` + +This helps have an overview of what is actively being worked on at any given time. + +### (Sentry Employees) How much time should be spent triaging? + +Generally, triaging should be distributed between the SDK team members as equally as possible. Every developer should +contribute to triaging as much as they can. + +Overall, developers should not spend more than 2h per day triaging & reproducing issues. If you find yourself spending +more time than this, bring this up with your manager to find ways to optimize this better. + +### (Sentry Employees) What about "inoffical" triaging? + +In addition to Github issues, you may also be pulled into triaging duty in other ways, e.g. via Discord , StackOverflow, +GitHub Discussions, or Slack. + +Generally, if non-trivial issues are raised this way, encourage the other parties to create issues on GitHub with as +much detail as possible, which also makes it easier for us to track the requests/issues. You should also include the +time you spend working on such issues in your general triaging time. + +### How to approach triaging an unknown issue? + +If you have no idea how to approach a given issue, there are a few general ways you could start: + +1. Ask for a more thorough reproduction. Often, an issue does not contain enough information for us to figure out what + is going on. Feel free to ask liberally for more information, if the provided information is not enough. +2. Ask users to enable debug logs (`Sentry.intit({ debug: true })`), and paste the logs for their app. This can contain + valuable information for debugging issues. +3. Ask colleagues who may have some experience with a category of issues. diff --git a/packages/feedback/src/core/integration.ts b/packages/feedback/src/core/integration.ts index 0cfd84061e67..8917644cebfe 100644 --- a/packages/feedback/src/core/integration.ts +++ b/packages/feedback/src/core/integration.ts @@ -1,6 +1,5 @@ import { getClient } from '@sentry/core'; import type { - FeedbackDialog, FeedbackInternalOptions, FeedbackModalIntegration, FeedbackScreenshotIntegration, @@ -56,7 +55,9 @@ export const buildFeedbackIntegration = ({ }: BuilderOptions): IntegrationFn< Integration & { attachTo(el: Element | string, optionOverrides?: OverrideFeedbackConfiguration): Unsubscribe; - createForm(optionOverrides?: OverrideFeedbackConfiguration): Promise; + createForm( + optionOverrides?: OverrideFeedbackConfiguration, + ): Promise>; createWidget(optionOverrides?: OverrideFeedbackConfiguration): ActorComponent; remove(): void; } @@ -179,7 +180,9 @@ export const buildFeedbackIntegration = ({ return integration as I; }; - const _loadAndRenderDialog = async (options: FeedbackInternalOptions): Promise => { + const _loadAndRenderDialog = async ( + options: FeedbackInternalOptions, + ): Promise> => { const screenshotRequired = options.enableScreenshot && isScreenshotSupported(); const [modalIntegration, screenshotIntegration] = await Promise.all([ _findIntegration('FeedbackModal', getModalIntegration, 'feedbackModalIntegration'), @@ -223,7 +226,7 @@ export const buildFeedbackIntegration = ({ throw new Error('Unable to attach to target element'); } - let dialog: FeedbackDialog | null = null; + let dialog: ReturnType | null = null; const handleClick = async (): Promise => { if (!dialog) { dialog = await _loadAndRenderDialog({ @@ -306,7 +309,9 @@ export const buildFeedbackIntegration = ({ * Creates a new Form which you can * Accepts partial options to override any options passed to constructor. */ - async createForm(optionOverrides: OverrideFeedbackConfiguration = {}): Promise { + async createForm( + optionOverrides: OverrideFeedbackConfiguration = {}, + ): Promise> { return _loadAndRenderDialog(mergeOptions(_options, optionOverrides)); }, diff --git a/packages/feedback/src/modal/components/Dialog.tsx b/packages/feedback/src/modal/components/Dialog.tsx index d691d0bd18c8..b711335a96c3 100644 --- a/packages/feedback/src/modal/components/Dialog.tsx +++ b/packages/feedback/src/modal/components/Dialog.tsx @@ -3,6 +3,7 @@ import type { FeedbackFormData, FeedbackInternalOptions } from '@sentry/types'; import { Fragment, h } from 'preact'; // eslint-disable-line @typescript-eslint/no-unused-vars import type { VNode } from 'preact'; import { useCallback, useMemo, useState } from 'preact/hooks'; + import { SUCCESS_MESSAGE_TIMEOUT } from '../../constants'; import { DialogHeader } from './DialogHeader'; import type { Props as HeaderProps } from './DialogHeader'; diff --git a/packages/feedback/src/modal/integration.tsx b/packages/feedback/src/modal/integration.tsx index 419a02328e0b..ab0b6418b8cd 100644 --- a/packages/feedback/src/modal/integration.tsx +++ b/packages/feedback/src/modal/integration.tsx @@ -1,13 +1,7 @@ import { getCurrentScope, getGlobalScope, getIsolationScope } from '@sentry/core'; -import type { - CreateDialogProps, - FeedbackDialog, - FeedbackFormData, - FeedbackModalIntegration, - IntegrationFn, - User, -} from '@sentry/types'; +import type { FeedbackFormData, FeedbackModalIntegration, IntegrationFn, User } from '@sentry/types'; import { h, render } from 'preact'; +import * as hooks from 'preact/hooks'; import { DOCUMENT } from '../constants'; import { Dialog } from './components/Dialog'; import { createDialogStyles } from './components/Dialog.css'; @@ -30,7 +24,7 @@ export const feedbackModalIntegration = ((): FeedbackModalIntegration => { name: 'FeedbackModal', // eslint-disable-next-line @typescript-eslint/no-empty-function setupOnce() {}, - createDialog: ({ options, screenshotIntegration, sendFeedback, shadow }: CreateDialogProps) => { + createDialog: ({ options, screenshotIntegration, sendFeedback, shadow }) => { const shadowRoot = shadow as unknown as ShadowRoot; const userKey = options.useSentryUser; const user = getUser(); @@ -39,7 +33,7 @@ export const feedbackModalIntegration = ((): FeedbackModalIntegration => { const style = createDialogStyles(); let originalOverflow = ''; - const dialog: FeedbackDialog = { + const dialog: ReturnType = { get el() { return el; }, @@ -66,7 +60,7 @@ export const feedbackModalIntegration = ((): FeedbackModalIntegration => { }, }; - const screenshotInput = screenshotIntegration && screenshotIntegration.createInput(h, dialog, options); + const screenshotInput = screenshotIntegration && screenshotIntegration.createInput({ h, hooks, dialog, options }); const renderContent = (open: boolean): void => { render( diff --git a/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx b/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx index 155cfac8e527..84528d2908a1 100644 --- a/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx +++ b/packages/feedback/src/screenshot/components/ScreenshotEditor.tsx @@ -1,12 +1,10 @@ -import type { FeedbackDialog, FeedbackInternalOptions } from '@sentry/types'; /* eslint-disable max-lines */ +import type { FeedbackInternalOptions, FeedbackModalIntegration } from '@sentry/types'; import type { ComponentType, VNode, h as hType } from 'preact'; -// biome-ignore lint: needed for preact -import { h } from 'preact'; // eslint-disable-line @typescript-eslint/no-unused-vars -import { useCallback, useEffect, useMemo, useRef, useState } from 'preact/hooks'; +import type * as Hooks from 'preact/hooks'; import { DOCUMENT, WINDOW } from '../../constants'; import { createScreenshotInputStyles } from './ScreenshotInput.css'; -import { useTakeScreenshot } from './useTakeScreenshot'; +import { useTakeScreenshotFactory } from './useTakeScreenshot'; const CROP_BUTTON_SIZE = 30; const CROP_BUTTON_BORDER = 3; @@ -15,8 +13,9 @@ const DPI = WINDOW.devicePixelRatio; interface FactoryParams { h: typeof hType; + hooks: typeof Hooks; imageBuffer: HTMLCanvasElement; - dialog: FeedbackDialog; + dialog: ReturnType; options: FeedbackInternalOptions; } @@ -62,17 +61,25 @@ const getContainedSize = (img: HTMLCanvasElement): Box => { return { startX: x, startY: y, endX: width + x, endY: height + y }; }; -export function makeScreenshotEditorComponent({ imageBuffer, dialog, options }: FactoryParams): ComponentType { +export function ScreenshotEditorFactory({ + h, // eslint-disable-line @typescript-eslint/no-unused-vars + hooks, + imageBuffer, + dialog, + options, +}: FactoryParams): ComponentType { + const useTakeScreenshot = useTakeScreenshotFactory({ hooks }); + return function ScreenshotEditor({ onError }: Props): VNode { - const styles = useMemo(() => ({ __html: createScreenshotInputStyles().innerText }), []); + const styles = hooks.useMemo(() => ({ __html: createScreenshotInputStyles().innerText }), []); - const canvasContainerRef = useRef(null); - const cropContainerRef = useRef(null); - const croppingRef = useRef(null); - const [croppingRect, setCroppingRect] = useState({ startX: 0, startY: 0, endX: 0, endY: 0 }); - const [confirmCrop, setConfirmCrop] = useState(false); + const canvasContainerRef = hooks.useRef(null); + const cropContainerRef = hooks.useRef(null); + const croppingRef = hooks.useRef(null); + const [croppingRect, setCroppingRect] = hooks.useState({ startX: 0, startY: 0, endX: 0, endY: 0 }); + const [confirmCrop, setConfirmCrop] = hooks.useState(false); - useEffect(() => { + hooks.useEffect(() => { WINDOW.addEventListener('resize', resizeCropper, false); }, []); @@ -99,7 +106,7 @@ export function makeScreenshotEditorComponent({ imageBuffer, dialog, options }: setCroppingRect({ startX: 0, startY: 0, endX: imageDimensions.width, endY: imageDimensions.height }); } - useEffect(() => { + hooks.useEffect(() => { const cropper = croppingRef.current; if (!cropper) { return; @@ -141,7 +148,7 @@ export function makeScreenshotEditorComponent({ imageBuffer, dialog, options }: DOCUMENT.addEventListener('mousemove', handleMouseMove); } - const makeHandleMouseMove = useCallback((corner: string) => { + const makeHandleMouseMove = hooks.useCallback((corner: string) => { return function (e: MouseEvent) { if (!croppingRef.current) { return; @@ -218,10 +225,10 @@ export function makeScreenshotEditorComponent({ imageBuffer, dialog, options }: } useTakeScreenshot({ - onBeforeScreenshot: useCallback(() => { + onBeforeScreenshot: hooks.useCallback(() => { (dialog.el as HTMLElement).style.display = 'none'; }, []), - onScreenshot: useCallback( + onScreenshot: hooks.useCallback( (imageSource: HTMLVideoElement) => { const context = imageBuffer.getContext('2d'); if (!context) { @@ -235,13 +242,13 @@ export function makeScreenshotEditorComponent({ imageBuffer, dialog, options }: }, [imageBuffer], ), - onAfterScreenshot: useCallback(() => { + onAfterScreenshot: hooks.useCallback(() => { (dialog.el as HTMLElement).style.display = 'block'; const container = canvasContainerRef.current; container && container.appendChild(imageBuffer); resizeCropper(); }, []), - onError: useCallback(error => { + onError: hooks.useCallback(error => { (dialog.el as HTMLElement).style.display = 'block'; onError(error); }, []), diff --git a/packages/feedback/src/screenshot/components/useTakeScreenshot.tsx b/packages/feedback/src/screenshot/components/useTakeScreenshot.tsx index bf50eab24283..a0e70247a007 100644 --- a/packages/feedback/src/screenshot/components/useTakeScreenshot.tsx +++ b/packages/feedback/src/screenshot/components/useTakeScreenshot.tsx @@ -1,8 +1,10 @@ -// biome-ignore lint/nursery/noUnusedImports: reason -import { h } from 'preact'; // eslint-disable-line @typescript-eslint/no-unused-vars -import { useEffect } from 'preact/hooks'; +import type * as Hooks from 'preact/hooks'; import { DOCUMENT, NAVIGATOR, WINDOW } from '../../constants'; +interface FactoryParams { + hooks: typeof Hooks; +} + interface Props { onBeforeScreenshot: () => void; onScreenshot: (imageSource: HTMLVideoElement) => void; @@ -10,36 +12,40 @@ interface Props { onError: (error: Error) => void; } -export const useTakeScreenshot = ({ onBeforeScreenshot, onScreenshot, onAfterScreenshot, onError }: Props): void => { - useEffect(() => { - const takeScreenshot = async (): Promise => { - onBeforeScreenshot(); - const stream = await NAVIGATOR.mediaDevices.getDisplayMedia({ - video: { - width: WINDOW.innerWidth * WINDOW.devicePixelRatio, - height: WINDOW.innerHeight * WINDOW.devicePixelRatio, - }, - audio: false, - // @ts-expect-error experimental flags: https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getDisplayMedia#prefercurrenttab - monitorTypeSurfaces: 'exclude', - preferCurrentTab: true, - selfBrowserSurface: 'include', - surfaceSwitching: 'exclude', - }); +type UseTakeScreenshot = ({ onBeforeScreenshot, onScreenshot, onAfterScreenshot, onError }: Props) => void; - const video = DOCUMENT.createElement('video'); - await new Promise((resolve, reject) => { - video.srcObject = stream; - video.onloadedmetadata = () => { - onScreenshot(video); - stream.getTracks().forEach(track => track.stop()); - resolve(); - }; - video.play().catch(reject); - }); - onAfterScreenshot(); - }; +export function useTakeScreenshotFactory({ hooks }: FactoryParams): UseTakeScreenshot { + return function useTakeScreenshot({ onBeforeScreenshot, onScreenshot, onAfterScreenshot, onError }: Props) { + hooks.useEffect(() => { + const takeScreenshot = async (): Promise => { + onBeforeScreenshot(); + const stream = await NAVIGATOR.mediaDevices.getDisplayMedia({ + video: { + width: WINDOW.innerWidth * WINDOW.devicePixelRatio, + height: WINDOW.innerHeight * WINDOW.devicePixelRatio, + }, + audio: false, + // @ts-expect-error experimental flags: https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getDisplayMedia#prefercurrenttab + monitorTypeSurfaces: 'exclude', + preferCurrentTab: true, + selfBrowserSurface: 'include', + surfaceSwitching: 'exclude', + }); - takeScreenshot().catch(onError); - }, []); -}; + const video = DOCUMENT.createElement('video'); + await new Promise((resolve, reject) => { + video.srcObject = stream; + video.onloadedmetadata = () => { + onScreenshot(video); + stream.getTracks().forEach(track => track.stop()); + resolve(); + }; + video.play().catch(reject); + }); + onAfterScreenshot(); + }; + + takeScreenshot().catch(onError); + }, []); + }; +} diff --git a/packages/feedback/src/screenshot/integration.ts b/packages/feedback/src/screenshot/integration.ts index 6edb1ce6a246..d4060c0071e2 100644 --- a/packages/feedback/src/screenshot/integration.ts +++ b/packages/feedback/src/screenshot/integration.ts @@ -1,25 +1,26 @@ -import type { - FeedbackDialog, - FeedbackInternalOptions, - FeedbackScreenshotIntegration, - IntegrationFn, -} from '@sentry/types'; +import type { FeedbackScreenshotIntegration, IntegrationFn } from '@sentry/types'; import type { Attachment } from '@sentry/types'; import type { h as hType } from 'preact'; +import type * as Hooks from 'preact/hooks'; import { DOCUMENT } from '../constants'; -import { makeScreenshotEditorComponent } from './components/ScreenshotEditor'; +import { ScreenshotEditorFactory } from './components/ScreenshotEditor'; export const feedbackScreenshotIntegration = ((): FeedbackScreenshotIntegration => { return { name: 'FeedbackScreenshot', // eslint-disable-next-line @typescript-eslint/no-empty-function setupOnce() {}, - createInput: (h: unknown, dialog: FeedbackDialog, options: FeedbackInternalOptions) => { + createInput: ({ h, hooks, dialog, options }) => { const imageBuffer = DOCUMENT.createElement('canvas'); return { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - input: makeScreenshotEditorComponent({ h: h as typeof hType, imageBuffer, dialog, options }) as any, + input: ScreenshotEditorFactory({ + h: h as typeof hType, + hooks: hooks as typeof Hooks, + imageBuffer, + dialog, + options, + }) as any, // eslint-disable-line @typescript-eslint/no-explicit-any value: async () => { const blob = await new Promise[0]>(resolve => { diff --git a/packages/node/src/integrations/tracing/koa.ts b/packages/node/src/integrations/tracing/koa.ts index 1fc85234fb76..071238eb7094 100644 --- a/packages/node/src/integrations/tracing/koa.ts +++ b/packages/node/src/integrations/tracing/koa.ts @@ -56,6 +56,7 @@ export const setupKoaErrorHandler = (app: { use: (arg0: (ctx: any, next: any) => await next(); } catch (error) { captureException(error); + throw error; } }); diff --git a/packages/node/src/sdk/initOtel.ts b/packages/node/src/sdk/initOtel.ts index 652c3826fa04..47c8879ae3e9 100644 --- a/packages/node/src/sdk/initOtel.ts +++ b/packages/node/src/sdk/initOtel.ts @@ -113,7 +113,11 @@ export function setupOtel(client: NodeClient): BasicTracerProvider { }), forceFlushTimeoutMillis: 500, }); - provider.addSpanProcessor(new SentrySpanProcessor()); + provider.addSpanProcessor( + new SentrySpanProcessor({ + timeout: client.getOptions().maxSpanWaitDuration, + }), + ); // Initialize the provider provider.register({ diff --git a/packages/node/src/types.ts b/packages/node/src/types.ts index d78e1761fd79..2c00302e2e64 100644 --- a/packages/node/src/types.ts +++ b/packages/node/src/types.ts @@ -74,6 +74,17 @@ export interface BaseNodeOptions { */ skipOpenTelemetrySetup?: boolean; + /** + * The max. duration in seconds that the SDK will wait for parent spans to be finished before discarding a span. + * The SDK will automatically clean up spans that have no finished parent after this duration. + * This is necessary to prevent memory leaks in case of parent spans that are never finished or otherwise dropped/missing. + * However, if you have very long-running spans in your application, a shorter duration might cause spans to be discarded too early. + * In this case, you can increase this duration to a value that fits your expected data. + * + * Defaults to 300 seconds (5 minutes). + */ + maxSpanWaitDuration?: number; + /** Callback that is executed when a fatal global error occurs. */ onFatalError?(this: void, error: Error): void; } diff --git a/packages/node/test/integration/transactions.test.ts b/packages/node/test/integration/transactions.test.ts index 75f5ef85a519..e3c9203ddaf5 100644 --- a/packages/node/test/integration/transactions.test.ts +++ b/packages/node/test/integration/transactions.test.ts @@ -633,4 +633,63 @@ describe('Integration | Transactions', () => { ]), ); }); + + it('allows to configure `maxSpanWaitDuration` to capture long running spans', async () => { + const transactions: TransactionEvent[] = []; + const beforeSendTransaction = jest.fn(event => { + transactions.push(event); + return null; + }); + + const now = Date.now(); + jest.useFakeTimers(); + jest.setSystemTime(now); + + const logs: unknown[] = []; + jest.spyOn(logger, 'log').mockImplementation(msg => logs.push(msg)); + + mockSdkInit({ + enableTracing: true, + beforeSendTransaction, + maxSpanWaitDuration: 100 * 60, + }); + + Sentry.startSpanManual({ name: 'test name' }, rootSpan => { + const subSpan = Sentry.startInactiveSpan({ name: 'inner span 1' }); + subSpan.end(); + + Sentry.startSpanManual({ name: 'inner span 2' }, innerSpan => { + // Child span ends after 10 min + setTimeout( + () => { + innerSpan.end(); + }, + 10 * 60 * 1_000, + ); + }); + + // root span ends after 99 min + setTimeout( + () => { + rootSpan.end(); + }, + 99 * 10 * 1_000, + ); + }); + + // Now wait for 100 mins + jest.advanceTimersByTime(100 * 60 * 1_000); + + expect(beforeSendTransaction).toHaveBeenCalledTimes(1); + expect(transactions).toHaveLength(1); + const transaction = transactions[0]!; + + expect(transaction.transaction).toEqual('test name'); + const spans = transaction.spans || []; + + expect(spans).toHaveLength(2); + + expect(spans[0]!.description).toEqual('inner span 1'); + expect(spans[1]!.description).toEqual('inner span 2'); + }); }); diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index 94e12bc019a8..d139044a713f 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -33,6 +33,7 @@ import { parseSpanDescription } from './utils/parseSpanDescription'; type SpanNodeCompleted = SpanNode & { span: ReadableSpan }; const MAX_SPAN_COUNT = 1000; +const DEFAULT_TIMEOUT = 300; // 5 min /** * A Sentry-specific exporter that converts OpenTelemetry Spans to Sentry Spans & Transactions. @@ -40,9 +41,11 @@ const MAX_SPAN_COUNT = 1000; export class SentrySpanExporter { private _flushTimeout: ReturnType | undefined; private _finishedSpans: ReadableSpan[]; + private _timeout: number; - public constructor() { + public constructor(options?: { timeout?: number }) { this._finishedSpans = []; + this._timeout = options?.timeout || DEFAULT_TIMEOUT; } /** Export a single span. */ @@ -103,7 +106,7 @@ export class SentrySpanExporter { */ private _cleanupOldSpans(spans = this._finishedSpans): void { this._finishedSpans = spans.filter(span => { - const shouldDrop = shouldCleanupSpan(span, 5 * 60); + const shouldDrop = shouldCleanupSpan(span, this._timeout); DEBUG_BUILD && shouldDrop && logger.log( diff --git a/packages/opentelemetry/src/spanProcessor.ts b/packages/opentelemetry/src/spanProcessor.ts index e3f604cab64b..71dab1359b94 100644 --- a/packages/opentelemetry/src/spanProcessor.ts +++ b/packages/opentelemetry/src/spanProcessor.ts @@ -65,9 +65,9 @@ function onSpanEnd(span: Span): void { export class SentrySpanProcessor implements SpanProcessorInterface { private _exporter: SentrySpanExporter; - public constructor() { + public constructor(options?: { timeout?: number }) { setIsSetup('SentrySpanProcessor'); - this._exporter = new SentrySpanExporter(); + this._exporter = new SentrySpanExporter(options); } /** diff --git a/packages/remix/src/index.client.tsx b/packages/remix/src/index.client.tsx index 4f0c9ab662ad..711fd3c2d2fc 100644 --- a/packages/remix/src/index.client.tsx +++ b/packages/remix/src/index.client.tsx @@ -17,7 +17,7 @@ export async function captureRemixServerException( err: unknown, name: string, request: Request, - isRemixV2: boolean, + isRemixV2?: boolean, ): Promise { DEBUG_BUILD && logger.warn( diff --git a/packages/remix/src/index.types.ts b/packages/remix/src/index.types.ts index 05bc6483218e..61088e370b45 100644 --- a/packages/remix/src/index.types.ts +++ b/packages/remix/src/index.types.ts @@ -22,7 +22,7 @@ export declare function captureRemixServerException( err: unknown, name: string, request: Request, - isRemixV2: boolean, + isRemixV2?: boolean, ): Promise; // This variable is not a runtime variable but just a type to tell typescript that the methods below can either come diff --git a/packages/remix/src/utils/errors.ts b/packages/remix/src/utils/errors.ts index 3c8943d2c107..92958c2c3eb3 100644 --- a/packages/remix/src/utils/errors.ts +++ b/packages/remix/src/utils/errors.ts @@ -172,7 +172,7 @@ export async function errorHandleDataFunction( // Remix v1 does not have a `handleError` function, so we capture all errors here. if (isRemixV2 ? isResponse(err) : true) { // eslint-disable-next-line @typescript-eslint/no-floating-promises - captureRemixServerException(err, name, args.request); + captureRemixServerException(err, name, args.request, true); } throw err; diff --git a/packages/remix/test/integration/app_v2/routes/click-error.tsx b/packages/remix/test/integration/app_v2/routes/click-error.tsx new file mode 100644 index 000000000000..3e42801b2cee --- /dev/null +++ b/packages/remix/test/integration/app_v2/routes/click-error.tsx @@ -0,0 +1,15 @@ +// Throw error on click +export default function ClickError() { + return ( +
+ +
+ ); +} diff --git a/packages/remix/test/integration/test/client/click-error.test.ts b/packages/remix/test/integration/test/client/click-error.test.ts new file mode 100644 index 000000000000..1e27cdcbdd69 --- /dev/null +++ b/packages/remix/test/integration/test/client/click-error.test.ts @@ -0,0 +1,38 @@ +import { expect, test } from '@playwright/test'; +import { Event } from '@sentry/types'; +import { getMultipleSentryEnvelopeRequests } from './utils/helpers'; + +const useV2 = process.env.REMIX_VERSION === '2'; + +test('should report a manually captured message on click with the correct stacktrace.', async ({ page }) => { + if (!useV2) { + test.skip(); + return; + } + + await page.goto('/click-error'); + + const promise = getMultipleSentryEnvelopeRequests(page, 2); + await page.click('#click-error'); + + const envelopes = await promise; + + const [_, errorEnvelope] = envelopes; + + expect(errorEnvelope.level).toBe('error'); + expect(errorEnvelope.sdk?.name).toBe('sentry.javascript.remix'); + + expect(errorEnvelope.exception?.values).toMatchObject([ + { + type: 'Error', + value: 'ClickError', + stacktrace: { frames: expect.any(Array) }, + mechanism: { type: 'instrument', handled: false }, + }, + ]); + + // Check the last frame of the stacktrace + const stacktrace = errorEnvelope.exception?.values[0]?.stacktrace?.frames; + + expect(stacktrace?.[stacktrace.length - 1].function).toBe('onClick'); +}); diff --git a/packages/solid/README.md b/packages/solid/README.md index 06ef8f810e93..7e042887854f 100644 --- a/packages/solid/README.md +++ b/packages/solid/README.md @@ -4,14 +4,14 @@

-# Official Sentry SDK for Solid +# Official Sentry SDK for Solid (EXPERIMENTAL) [![npm version](https://img.shields.io/npm/v/@sentry/solid.svg)](https://www.npmjs.com/package/@sentry/solid) [![npm dm](https://img.shields.io/npm/dm/@sentry/solid.svg)](https://www.npmjs.com/package/@sentry/solid) [![npm dt](https://img.shields.io/npm/dt/@sentry/solid.svg)](https://www.npmjs.com/package/@sentry/solid) -This SDK is considered **experimental and in an alpha state**. It may experience breaking changes. Please reach out on -[GitHub](https://github.com/getsentry/sentry-javascript/issues/new/choose) if you have any feedback or concerns. This +This SDK is considered ⚠️ **experimental and in an alpha state**. It may experience breaking changes. Please reach out +on [GitHub](https://github.com/getsentry/sentry-javascript/issues/new/choose) if you have any feedback or concerns. This SDK currently only supports [Solid](https://www.solidjs.com/) and is not yet officially compatible with [Solid Start](https://start.solidjs.com/). @@ -20,25 +20,22 @@ SDK currently only supports [Solid](https://www.solidjs.com/) and is not yet off The Solid Router instrumentation uses the Solid Router library to create navigation spans to ensure you collect meaningful performance data about the health of your page loads and associated requests. -Add `Sentry.solidRouterBrowserTracingIntegration` instead of the regular `Sentry.browserTracingIntegration` and provide -the hooks it needs to enable performance tracing: +Add `solidRouterBrowserTracingIntegration` instead of the regular `Sentry.browserTracingIntegration`. -`useBeforeLeave` from `@solidjs/router` -`useLocation` from `@solidjs/router` +Make sure `solidRouterBrowserTracingIntegration` is initialized by your `Sentry.init` call. Otherwise, the routing +instrumentation will not work properly. -Make sure `Sentry.solidRouterBrowserTracingIntegration` is initialized by your `Sentry.init` call, before you wrap -`Router`. Otherwise, the routing instrumentation may not work properly. - -Wrap `Router`, `MemoryRouter` or `HashRouter` from `@solidjs/router` using `Sentry.withSentryRouterRouting`. This -creates a higher order component, which will enable Sentry to reach your router context. +Wrap `Router`, `MemoryRouter` or `HashRouter` from `@solidjs/router` using `withSentryRouterRouting`. This creates a +higher order component, which will enable Sentry to reach your router context. ```js import * as Sentry from '@sentry/solid'; -import { Route, Router, useBeforeLeave, useLocation } from '@solidjs/router'; +import { solidRouterBrowserTracingIntegration, withSentryRouterRouting } from '@sentry/solid/solidrouter'; +import { Route, Router } from '@solidjs/router'; Sentry.init({ dsn: '__PUBLIC_DSN__', - integrations: [Sentry.solidRouterBrowserTracingIntegration({ useBeforeLeave, useLocation })], + integrations: [solidRouterBrowserTracingIntegration()], tracesSampleRate: 1.0, // Capture 100% of the transactions }); diff --git a/packages/solid/package.json b/packages/solid/package.json index efb0e14236bb..84ce5b5a4c3d 100644 --- a/packages/solid/package.json +++ b/packages/solid/package.json @@ -12,30 +12,35 @@ "files": [ "cjs", "esm", - "types", - "types-ts3.8" + "index.d.ts", + "index.d.ts.map", + "solidrouter.d.ts", + "solidrouter.d.ts.map" ], "main": "build/cjs/index.js", "module": "build/esm/index.js", - "types": "build/types/index.d.ts", + "types": "build/index.d.ts", "exports": { "./package.json": "./package.json", ".": { "import": { - "types": "./build/types/index.d.ts", + "types": "./build/index.d.ts", "default": "./build/esm/index.js" }, "require": { - "types": "./build/types/index.d.ts", + "types": "./build/index.d.ts", "default": "./build/cjs/index.js" } - } - }, - "typesVersions": { - "<4.9": { - "build/types/index.d.ts": [ - "build/types-ts3.8/index.d.ts" - ] + }, + "./solidrouter": { + "import": { + "types": "./build/solidrouter.d.ts", + "default": "./build/esm/solidrouter.js" + }, + "require": { + "types": "./build/solidrouter.d.ts", + "default": "./build/cjs/solidrouter.js" + } } }, "publishConfig": { @@ -48,10 +53,16 @@ "@sentry/utils": "8.11.0" }, "peerDependencies": { + "@solidjs/router": "^0.13.4", "solid-js": "^1.8.4" }, + "peerDependenciesMeta": { + "@solidjs/router": { + "optional": true + } + }, "devDependencies": { - "@solidjs/router": "^0.13.5", + "@solidjs/router": "^0.13.4", "@solidjs/testing-library": "0.8.5", "@testing-library/jest-dom": "^6.4.5", "@testing-library/user-event": "^14.5.2", @@ -62,9 +73,8 @@ "build": "run-p build:transpile build:types", "build:dev": "yarn build", "build:transpile": "rollup -c rollup.npm.config.mjs", - "build:types": "run-s build:types:core build:types:downlevel", + "build:types": "run-s build:types:core", "build:types:core": "tsc -p tsconfig.types.json", - "build:types:downlevel": "yarn downlevel-dts build/types build/types-ts3.8 --to ts3.8", "build:watch": "run-p build:transpile:watch build:types:watch", "build:dev:watch": "yarn build:watch", "build:transpile:watch": "rollup -c rollup.npm.config.mjs --watch", diff --git a/packages/solid/rollup.npm.config.mjs b/packages/solid/rollup.npm.config.mjs index 84a06f2fb64a..b044fda38c75 100644 --- a/packages/solid/rollup.npm.config.mjs +++ b/packages/solid/rollup.npm.config.mjs @@ -1,3 +1,7 @@ import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollup-utils'; -export default makeNPMConfigVariants(makeBaseNPMConfig()); +export default makeNPMConfigVariants( + makeBaseNPMConfig({ + entrypoints: ['src/index.ts', 'src/solidrouter.ts'], + }), +); diff --git a/packages/solid/src/index.ts b/packages/solid/src/index.ts index 48aee7358776..8a05327bcf0b 100644 --- a/packages/solid/src/index.ts +++ b/packages/solid/src/index.ts @@ -2,5 +2,4 @@ export * from '@sentry/browser'; export { init } from './sdk'; -export * from './solidrouter'; export * from './errorboundary'; diff --git a/packages/solid/src/solidrouter.ts b/packages/solid/src/solidrouter.ts index d22e7afb889a..2f343cfe9d7c 100644 --- a/packages/solid/src/solidrouter.ts +++ b/packages/solid/src/solidrouter.ts @@ -12,55 +12,21 @@ import { getClient, } from '@sentry/core'; import type { Client, Integration, Span } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import type { + BeforeLeaveEventArgs, + HashRouter, + MemoryRouter, + RouteSectionProps, + Router as BaseRouter, + StaticRouter, +} from '@solidjs/router'; +import { useBeforeLeave, useLocation } from '@solidjs/router'; import { createEffect, mergeProps, splitProps } from 'solid-js'; import type { Component, JSX, ParentProps } from 'solid-js'; import { createComponent } from 'solid-js/web'; -import { DEBUG_BUILD } from './debug-build'; - -// Vendored solid router types so that we don't need to depend on solid router. -// These are not exhaustive and loose on purpose. -interface Location { - pathname: string; -} - -interface BeforeLeaveEventArgs { - from: Location; - to: string | number; -} - -interface RouteSectionProps { - location: Location; - data?: T; - children?: JSX.Element; -} - -// eslint-disable-next-line @typescript-eslint/no-explicit-any -type RouteDefinition = { - path?: S; - children?: RouteDefinition | RouteDefinition[]; - component?: Component>; -}; - -interface RouterProps { - base?: string; - root?: Component; - children?: JSX.Element | RouteDefinition | RouteDefinition[]; -} - -interface SolidRouterOptions { - useBeforeLeave: UserBeforeLeave; - useLocation: UseLocation; -} - -type UserBeforeLeave = (listener: (e: BeforeLeaveEventArgs) => void) => void; -type UseLocation = () => Location; const CLIENTS_WITH_INSTRUMENT_NAVIGATION = new WeakSet(); -let _useBeforeLeave: UserBeforeLeave; -let _useLocation: UseLocation; - function handleNavigation(location: string): void { const client = getClient(); if (!client || !CLIENTS_WITH_INSTRUMENT_NAVIGATION.has(client)) { @@ -98,12 +64,12 @@ function withSentryRouterRoot(Root: Component): Component { + useBeforeLeave(({ to }: BeforeLeaveEventArgs) => { // `to` could be `-1` if the browser back-button was used handleNavigation(to.toString()); }); - const location = _useLocation(); + const location = useLocation(); createEffect(() => { const name = location.pathname; const rootSpan = getActiveRootSpan(); @@ -130,21 +96,17 @@ function withSentryRouterRoot(Root: Component): Component[0] & SolidRouterOptions, + options: Parameters[0] = {}, ): Integration { const integration = browserTracingIntegration({ ...options, instrumentNavigation: false, }); - const { instrumentNavigation = true, useBeforeLeave, useLocation } = options; + const { instrumentNavigation = true } = options; return { ...integration, - setup() { - _useBeforeLeave = useBeforeLeave; - _useLocation = useLocation; - }, afterAllSetup(client) { integration.afterAllSetup(client); @@ -155,19 +117,13 @@ export function solidRouterBrowserTracingIntegration( }; } +type RouterType = typeof BaseRouter | typeof HashRouter | typeof MemoryRouter | typeof StaticRouter; + /** * A higher-order component to instrument Solid Router to create navigation spans. */ -export function withSentryRouterRouting(Router: Component): Component { - if (!_useBeforeLeave || !_useLocation) { - DEBUG_BUILD && - logger.warn(`solidRouterBrowserTracingIntegration was unable to wrap Solid Router because of one or more missing hooks. - useBeforeLeave: ${_useBeforeLeave}. useLocation: ${_useLocation}.`); - - return Router; - } - - const SentryRouter = (props: RouterProps): JSX.Element => { +export function withSentryRouterRouting(Router: RouterType): RouterType { + const SentryRouter = (props: Parameters[0]): JSX.Element => { const [local, others] = splitProps(props, ['root']); // We need to wrap root here in case the user passed in their own root const Root = withSentryRouterRoot(local.root ? local.root : SentryDefaultRoot); diff --git a/packages/solid/test/solidrouter.test.tsx b/packages/solid/test/solidrouter.test.tsx index a268a8ee50ed..029b90794b70 100644 --- a/packages/solid/test/solidrouter.test.tsx +++ b/packages/solid/test/solidrouter.test.tsx @@ -7,7 +7,8 @@ import { getCurrentScope, setCurrentClient, } from '@sentry/core'; -import { MemoryRouter, Navigate, Route, createMemoryHistory, useBeforeLeave, useLocation } from '@solidjs/router'; +import type { MemoryHistory } from '@solidjs/router'; +import { MemoryRouter, Navigate, Route, createMemoryHistory } from '@solidjs/router'; import { render } from '@solidjs/testing-library'; import { vi } from 'vitest'; @@ -17,7 +18,7 @@ import { solidRouterBrowserTracingIntegration, withSentryRouterRouting } from '. // solid router uses `window.scrollTo` when navigating vi.spyOn(global, 'scrollTo').mockImplementation(() => {}); -const renderRouter = (SentryRouter, history) => +const renderRouter = (SentryRouter: typeof MemoryRouter, history: MemoryHistory) => render(() => (
Home
} /> @@ -58,7 +59,7 @@ describe('solidRouterBrowserTracingIntegration', () => { setCurrentClient(client); client.on('spanStart', span => spanStartMock(spanToJSON(span))); - client.addIntegration(solidRouterBrowserTracingIntegration({ useBeforeLeave, useLocation })); + client.addIntegration(solidRouterBrowserTracingIntegration()); const history = createMemoryHistory(); history.set({ value: '/' }); @@ -86,8 +87,6 @@ describe('solidRouterBrowserTracingIntegration', () => { client.addIntegration( solidRouterBrowserTracingIntegration({ instrumentPageLoad: false, - useBeforeLeave, - useLocation, }), ); const SentryRouter = withSentryRouterRouting(MemoryRouter); @@ -124,7 +123,7 @@ describe('solidRouterBrowserTracingIntegration', () => { client.on('spanStart', span => { spanStartMock(spanToJSON(span)); }); - client.addIntegration(solidRouterBrowserTracingIntegration({ useBeforeLeave, useLocation })); + client.addIntegration(solidRouterBrowserTracingIntegration()); const SentryRouter = withSentryRouterRouting(MemoryRouter); const history = createMemoryHistory(); @@ -155,8 +154,6 @@ describe('solidRouterBrowserTracingIntegration', () => { client.addIntegration( solidRouterBrowserTracingIntegration({ instrumentNavigation: false, - useBeforeLeave, - useLocation, }), ); const SentryRouter = withSentryRouterRouting(MemoryRouter); @@ -188,7 +185,7 @@ describe('solidRouterBrowserTracingIntegration', () => { client.on('spanStart', span => { spanStartMock(spanToJSON(span)); }); - client.addIntegration(solidRouterBrowserTracingIntegration({ useBeforeLeave, useLocation })); + client.addIntegration(solidRouterBrowserTracingIntegration()); const SentryRouter = withSentryRouterRouting(MemoryRouter); const history = createMemoryHistory(); diff --git a/packages/solid/tsconfig.types.json b/packages/solid/tsconfig.types.json index 65455f66bd75..49a8c984fcc6 100644 --- a/packages/solid/tsconfig.types.json +++ b/packages/solid/tsconfig.types.json @@ -5,6 +5,6 @@ "declaration": true, "declarationMap": true, "emitDeclarationOnly": true, - "outDir": "build/types" + "outDir": "build" } } diff --git a/packages/types/src/feedback/index.ts b/packages/types/src/feedback/index.ts index aa34d1a574b7..b26cb66ed07d 100644 --- a/packages/types/src/feedback/index.ts +++ b/packages/types/src/feedback/index.ts @@ -1,6 +1,5 @@ import type { Attachment } from '../attachment'; import type { Integration } from '../integration'; - import type { FeedbackCallbacks, FeedbackGeneralConfiguration, @@ -22,8 +21,13 @@ export interface FeedbackInternalOptions FeedbackTextConfiguration, FeedbackCallbacks {} +type Hooks = unknown; type HTMLElement = unknown; -export interface FeedbackDialog { +type HType = unknown; +type ShadowRoot = unknown; +type VNode = unknown; + +type FeedbackDialog = { /** * The HTMLElement that is containing all the form content */ @@ -50,10 +54,21 @@ export interface FeedbackDialog { * Close/Hide the dialog & form inside it */ close: () => void; +}; + +interface FeedbackScreenshotInput { + /** + * The preact component + */ + input: (props: { onError: (error: Error) => void }) => VNode; + + /** + * The image/screenshot bytes + */ + value: () => Promise; } -type ShadowRoot = unknown; -export interface CreateDialogProps { +interface CreateDialogProps { options: FeedbackInternalOptions; screenshotIntegration: FeedbackScreenshotIntegration | undefined; sendFeedback: SendFeedback; @@ -63,22 +78,12 @@ export interface FeedbackModalIntegration extends Integration { createDialog: (props: CreateDialogProps) => FeedbackDialog; } -type HType = unknown; -type VNode = unknown; +interface CreateInputProps { + h: HType; + hooks: Hooks; + dialog: FeedbackDialog; + options: FeedbackInternalOptions; +} export interface FeedbackScreenshotIntegration extends Integration { - createInput: ( - h: HType, - dialog: FeedbackDialog, - options: FeedbackInternalOptions, - ) => { - /** - * The preact component - */ - input: (props: { onError: (error: Error) => void }) => VNode; - - /** - * The image/screenshot bytes - */ - value: () => Promise; - }; + createInput: (props: CreateInputProps) => FeedbackScreenshotInput; } diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 257620abc9f7..a7cf18056eb6 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -77,7 +77,6 @@ export type { } from './profiling'; export type { ReplayEvent, ReplayRecordingData, ReplayRecordingMode } from './replay'; export type { - FeedbackDialog, FeedbackEvent, FeedbackFormData, FeedbackInternalOptions, @@ -85,7 +84,6 @@ export type { FeedbackScreenshotIntegration, SendFeedback, SendFeedbackParams, - CreateDialogProps, UserFeedback, } from './feedback'; export type { QueryParams, Request, SanitizedRequestData } from './request'; diff --git a/yarn.lock b/yarn.lock index 6ed78f795847..e1fc4cb3cf5f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8774,10 +8774,10 @@ resolved "https://registry.yarnpkg.com/@socket.io/component-emitter/-/component-emitter-3.1.0.tgz#96116f2a912e0c02817345b3c10751069920d553" integrity sha512-+9jVqKhRSpsc591z5vX+X5Yyw+he/HCB4iQ/RYxw35CEPaY1gnsNE43nf9n9AaYjAQrTiI/mOwKUKdUs9vf7Xg== -"@solidjs/router@^0.13.5": - version "0.13.5" - resolved "https://registry.yarnpkg.com/@solidjs/router/-/router-0.13.5.tgz#62ee37f63d2b5f74937903d64d04ec9a2b4223cf" - integrity sha512-I/bR5ZHCz2Dx80qL+6uGwSdclqXRqoT49SJ5cvLbOuT3HnYysSIxSfULCTWUMLFVcgPh5GrdHV6KwEoyrbPZZA== +"@solidjs/router@^0.13.4": + version "0.13.6" + resolved "https://registry.yarnpkg.com/@solidjs/router/-/router-0.13.6.tgz#210ca2761d4bf294f06ac0f9e25c16fafdabefac" + integrity sha512-CdpFsBYoiJ/FQ4wZIamj3KEFRkmrYu5sVXM6PouNkmSENta1YJamsm9wa/VjaPmkw2RsnDnO0UvZ705v6EgOXQ== "@solidjs/testing-library@0.8.5": version "0.8.5"