[pull] master from getsentry:master #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=60&v=4){kind=small}
Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
This PR adds support for Shopify Hydrogen applications running on MiniOxygen runtime. ### What's updated **Issue with `node:async_hooks`** (#5610 (comment)) - `@sentry/cloudflare` has a new export path called `./request` to import only the request wrapper without involving `async.ts` which imports `node:async_hooks`. That fixes the build problems on the MiniOxygen environment. **Issue with imported empty modules on MiniOxygen** (Related: vitejs/vite#10612) - `@sentry/remix` has a new export path called `./cloudflare` to fix empty methods in the imported modules. The problem there seemed to be the re-exports that are somehow not resolved. So with this, I also updated the file structure of Remix SDK to separate the client/server/cloudflare code with their own per-folder `index` files. **Issue having `loader` and `action` spans** - This is currently [not supported](https://github.com/justindsmith/opentelemetry-instrumentations-js/tree/main/packages/instrumentation-remix#cloudflare-worker-warning) on `cloudflare` environments on the auto-instrumented flow. To support them, I re-introduced the span creation flow we were using before migration. That's a step back from #15074, but by default this logic is disabled. It's enabled by default when `instrumentBuild` is imported from `@sentry/remix/cloudflare`. **The root server spans are not parameterised** That's because we don't have the Remix routes information on `@sentry/cloudflare`'s request wrapper. I have tried updating the root span's name inside `@sentry/remix/cloudflare`'s logic and it works. But I have left it out for now, not to make this PR even more difficult to review.
and only warn about missing onRequestError when an instrumentation file exists
Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
…andler-2 fix(nextjs/instrumentation): warn about missing onRequestError handler
…/e2e-tests/test-applications/remix-hydrogen (#15489)
Jest didn't like usages of `import.meta.url` but now we're using Vitest, these hacks can be removed. Our Rollup config already converts this correctly when building our CommonJs output. --------- Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>
Before: `7.549 s` After: `Duration 3.10s (transform 2.32s, setup 0ms, collect 13.39s, tests 599ms, environment 4.11s, prepare 945ms)`
Jest is not being used, we can remove all jest related code from this package
…ons (#15283) This started to be an issue after we added support for descendant React Router routes. React Router does not limit users to only one way to declare routes. Currently, we provide instrumentation for 3 ways of declaring RR routes: | React Router v6/v7 | Sentry | |-----------------------|----------------------------------| | `useRoutes` | `Sentry.wrapUseRoutes` | | `<Routes />` | `Sentry.withReactRouterRouting` | | `createBrowserRouter` | `Sentry.wrapCreateBrowserRouter` | | `createHashRouter` | `Sentry.wrapCreateBrowserRouter` | | `createMemoryRouter` | `Sentry.wrapCreateMemoryRouter` | When users decide to declare their routes using more than one of these options together, we risk losing context while rebuilding parameterised span names. This PR adds: - Proper support and context sharing for cross-usage of route declarations - E2E tests for pairs of 2 and all three possible methods used together to declare descendant routes. - Adds another recursion break for the path rebuild method for cases when the rebuilt path has not changed in a deeper recursive call. (This fixes #15279, together with the updates mentioned above)
Before: `6.437 s` After: `1.23s` The first thing we did was migrate the `pretest.ts` script to use a vitest global setup instead. This is cleaner, and should ensure it never gets lost in refactors either. After that we did the conversion. The trickiest thing to change was `packages/gatsby/test/gatsby-node.test.ts`, as `gatsby-node.js` is a CJS file. This is tricky to patch properly with vitest for the spy. To get around this, we monkeypatch `Module._load` temporarily to load a stub which we defined in `vi.hoisted` (which hoists it to the top so it's always available).
This PR adds `jsx,tsx,astro,vue` formatting to prettier. Svelte needs a prettier plugin and I couldn't get it working within 10 minutes so gave up for now.
These are no longer required since #15461
Before submitting a pull request, please take a look at our [Contributing](https://github.com/getsentry/sentry-javascript/blob/master/CONTRIBUTING.md) guidelines and verify: - [ ] If you've added code that should be tested, please add tests. - [ ] Ensure your code lints and the test suite passes (`yarn lint`) & (`yarn test`). ### Description I noticed that Nuxt 300-400 status codes from the app are being reported to Sentry unnecessarily. There appears to be a guard against doing this on the server side errors but client side errors like 404 or 401s are being reported to Sentry. I have simply added the same guard on the client side error reporter for the Nuxt integration of Sentry.
…e2e-tests/test-applications/solidstart (#15495)
This PR: - Modifies all tests to: - Use Vitest imports - Use async rather than callback - Changes test runner and test server to be async - `runner.start(done)` becomes `await runner.start().completed()` - `createTestServer()` close function now throws any errors that occurred
We should not use Vitest globals except where they are required. This PR: - Disables `globals` in our root `vite.config.ts` - Fixes missing imports - Sets `globals: true` for React since there are load of test failures without it and I couldn't immedialy work out why
This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #15473 Co-authored-by: s1gr1d <32902192+s1gr1d@users.noreply.github.com>
…ation` (#15434) Deprecate and rename the `inboundFiltersIntegration` to `eventFiltersIntegration` to improve the integration name and avoid ambiguity with the product.
I experimented with a bunch of different approaches to register log flushing implementations, but I found the cleanest and most performant solution was to have log flushing logic live in clients. This refactors the browser client to do that, and adds tests for all of our flushing logic everywhere. The bundle size increase is not ideal, but I recognize we have no choice. After this gets merged in, I'll add the console logging integration to core. I'll also add some quick integrations for pino and winston.
meta(changelog): Update changelog for 9.10.1
[Gitflow] Merge master into develop
In preparation for OpenTelemetry v2, which targets ES2022, we bump the checks to ES2022 for node-based packages.
As `require` is supported in ESM as well from [Node 20.19.0](https://nodejs.org/en/blog/release/v20.19.0) onwards, the check does not work anymore. However, `module` is not available in ESM. Also mentioned in this comment: #14202 (comment) [Node: Compatibility with CommonJS](https://nodejs.org/docs/latest-v15.x/api/esm.html#esm_interoperability_with_commonjs) [Bun: Using require](https://bun.sh/docs/runtime/modules#using-require)
Ref getsentry/projects#837 for for not truncating error messages and letting relay do that. Kinda undoes #8593 Should be merged after #15819 because otherwise, we keep humongous strings in memory.
The bottleneck of many of the tasks written down in our Node SDK performance improvement task #15861 is `parseUrl`, defined here: https://github.com/getsentry/sentry-javascript/blob/3d63621714b31c1ea4c2ab2d90d5684a36805a43/packages/core/src/utils-hoist/url.ts#L17 We created #15767 to track removal of `parseUrl`. See more details in the GH issue. While working on tasks for #15767, I initially PR'd #15768, which introduced a `parseStringToURL` method as a replacement for `parseUrl`. While trying to implement that method though I realized `parseStringToURL` has a lot of downsides. This PR removes `parseStringToURL` in favor of a new `parseStringToURLObject`. Given `parseStringToURL` was never exported by the core SDK, this is not a breaking change. To understand `parseStringToURLObject`, let's first look at it's method signature: ```ts function parseStringToURLObject(url: string, urlBase?: string | URL | undefined): URLObject | undefined ``` `parseStringToURLObject` takes a string URL and turns it into a `URLObject`, that is typed like so: ```ts type RelativeURL = { isRelative: true; pathname: URL['pathname']; search: URL['search']; hash: URL['hash']; }; type URLObject = RelativeURL | URL; ``` the JavaScript URL built-in does not handle relative URLs, so we need to use a separate object to to track relative URLs. Luckily it's pretty small surface area, we only need to worry about `pathname`, `search`, and `hash`. For ease of usage of this function, I also introduced a `isURLObjectRelative` helper. This will make sure that users can handle both relative and absolute URLs with ease. Given `packages/core/src/fetch.ts` was using `parseStringToURL`, I refactored that file to use `parseStringToURLObject`. The change feels way better to me, much easier to read and understand what is going on.
This used to build into `/lib` which is not covered by nx (it was changed there but slightly incorrectly, covering `/build/lib` instead of `/lib`). But overall, the way to go here is to just use our standard structure, which is `build/`, then this should all just work :D
…5930) Since we dropped support for Remix v1, we're not wrapping Remix `root`s with `ErrorBoundary` anymore. Fixes https://github.com/orgs/getsentry/projects/31/views/19?pane=issue&itemId=103941554&issue=getsentry%7Csentry-javascript%7C15883 Docs PR: getsentry/sentry-docs#13170
When running some unit tests locally, I noticed that the coverage summary contained files like `vite.config.ts` or `.eslintrc` as well as transpiled code in `build`. These files shouldn't be in the coverage reports and neither have an influence on the overall coverage score.
…ct` span (#15943) Adds an `http.redirect_count` attribute to `browser.redirect` spans. The count is taken from the `navigation` performance entry and it describes the number of times a redirect happened. Two caveats: - we can't detect more about redirects (e.g. the location the browser was redirected to) - this only works for same-origin redirects. [According to MDN](https://developer.mozilla.org/en-US/docs/Web/API/PerformanceNavigationTiming/redirectCount#value), cross-origin redirects are not taken into account.
…15957) Temporarily add a span attribute for the previous trace to get trace links working in the EAP-based trace view. This needs to be removed once EAP properly supports span links.
Turns out we never added the release registry entry for nuxt. The corresponding [folder exists in the release registry repo](https://github.com/getsentry/sentry-release-registry/tree/master/packages/npm/%40sentry/nuxt).
Should wait on getsentry/sentry-release-registry#187 before merging.
…y craft entry (#15964) Corresponding entry in sentry-release-registry: https://github.com/getsentry/sentry-release-registry/tree/master/packages/npm/%40sentry/tanstackstart-react
…ap settings (#15859) There is a total of 6 settings in Nuxt where you can disable/enable your source maps. This is not only difficult to understand when writing and maintaining the logic for this. It may also be hard to debug stuff for users.  There is this discussion around Nuxt source maps: #15028 And this issue: #15160 A problem with the current setup was not only the difficulty to understand all of this but also it sometimes just overwrote stuff you didn't want it to. Like in the default case of Nuxt, `sourcemap.server` is `true`, but as `nitro.rollupConfig.output.sourcemap` is always undefined, Sentry would overwrite this setting to `'hidden'`, even though the source maps were enabled already. --- The only two relevant options in Nuxt are the [root-level sourcemap options](https://nuxt.com/docs/guide/going-further/debugging#sourcemaps). Those settings will propagate to nitro, vite and rollup. As we overwrite the source maps setting if the setting is undefined, only the basic Nuxt source map settings (linked above) are taken into account for that from now on.
Adds `consoleLoggingIntegration` integration that takes calls to
`console.X` and flushes them as logs. For now this is an opt-in
integration. We can evaluate it's default status at a later point.
```js
import * as Sentry from '@sentry/browser'; // or any other supported sdk
Sentry.init({
// send console.log, console.error, and console.warn calls as logs to Sentry
integrations: [Sentry.consoleLoggingIntegration({ levels: ['log', 'error', 'warn'] })],
});
```
## Notes
In general we have inconsistencies with console instrumentation across
our different SDKs.
- In browser we have console instrumentation as part of the
`Breadcrumbs` integration.
- In node we have console instrumentation that generates breadcrumbs in
the `Console` integration
- In core we have console instrumentation, which generates errors and
messages via the `CaptureConsole` integration
For now because logs are experimental, making it a standalone
integration feels fine to me. As we get closer to GA status for logs
(and there is a distinct path for breadcrumbs) we should evaluate how
all of our console integrations are structured.
Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
meta: Update CHANGELOG for 9.11.0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )