meta(changelog): Update changelog for 8.39.0 #14361
Merged
Conversation
This file contains hidden or 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
[Gitflow] Merge master into develop
Noticed while debugging some test problems, that if axios throws an error (e.g. you get a 500 error), jest cannot serialize the error because it contains recursive data, leading to super hard to debug error messages. This makes our node integration test runner more resilient by normalizing errors we get there to ensure circular references are resolved. ## Update While working on this, I found some further, actually more serious problems - some tests were simply not running at all. Especially all the session aggregrate tests were simply not being run and just "passed" accidentally. The core problem there was that the path to the scenario was incorrect, so nothing was even started, plus some things where we seemed to catch errors and still pass (?? I do not think I fixed all of the issues there, but at least some of them...) Now, we assert that the scenario file actually exists. Plus, instead of setting `.expectError()` on the whole runner, you now have to pass this to a specific `makeRequest` call as an optional option, and it will also assert if we expect an error _but do not get one_. This way, the tests can be more explicit and clear in what they do.
Oops, #14245 broke our remix integration tests. Instead of re-exporting this from a path (which is not resolved nicely etc. and not reflected by our dependency graph) we can just inline the two methods we need here, they are not too complicated.
We support v8 but only test v10. By adding a (very basic) e2e test app for v8 of nestjs we can ensure this continues to work (as long as we want to support it). Also bumps nestjs dev deps to hopefully fix some security warnings for outdates deps (e.g. https://github.com/getsentry/sentry-javascript/security/dependabot/376). Note for the future: There seems to be some problem with the node-integration-tests and module resolution. As soon as the nestjs version there matches the one from the nestjs package - which means it will hoist the dependency to the workspace-level node_modules folder - the tests start failing weirdly. For now, I "fixed" this by using a different version of nestjs in node-integration-tests from nestjs. Since we want to get rid of the node-specific part there anyhow in v9, I figured it's not worth it to put more work into fixing this properly...
…ests (#13746) This PR started out as trying to fix capturing request bodies for Koa. Some investigation later, we found out that the fundamental problem was that we relied on the request body being on `request.body`, which is non-standard and thus does not necessarily works. It seems that in express this works because it under the hood writes the body there, but this is non-standard and rather undefined behavior. For other frameworks (e.g. Koa and probably more) this did not work, the body was not on the request and thus never captured. We also had no test coverage for this overall. This PR ended up doing a few things: * Add tests for this for express and koa * Streamline types for `sdkProcessingMetadata` - this used to be `any`, which lead to any usage of this not really being typed at all. I added proper types for this now. * Generic extraction of the http request body in the http instrumentation - this should now work for any node framework Most importantly, I opted to not force this into the existing, rather complicated and hard to follow request data integration flow. This used to take an IsomorphicRequest and then did a bunch of conversion etc. Since now in Node, we always have the same, proper http request (for any framework, because this always goes through http instrumentation), we can actually streamline this and normalize this properly at the time where we set this. So with this PR, we set a `normalizedRequest` which already has the url, headers etc. set in a way that we need it/it makes sense. Additionally, the parsed & stringified request body will be set on this too. If this normalized request is set in sdkProcessingMetadata, we will use it as source of truth instead of the plain `request`. (Note that we still need the plain request for some auxiliary data that is non-standard, e.g. `request.user`). For the body parsing itself, we monkey-patch `req.on('data')`. this way, we ensure to not add more handlers than a user has, and we only extract the body if the user is extracting it anyhow, ensuring we do not alter behavior. Closes #13722 --------- Co-authored-by: Luca Forstner <[email protected]>
This PR fixes the electron crash observed in #13978 I am not entirely sure as to why this causes a sigabrt, so I am working around the issue (obtaining a coredump out of electron did not seem trivial and my knowledge around electron isn't very extensive). The v8 options class and the constants are exposed correctly, I ruled that out, however the crash still seems to happen when they are used in this specific signature. In order to have this running with electron, users will require to use the electron/rebuild package, which is the recommended approach by electron that rebuilds native node addons by providing the correct abi headers for the electron version the user is running. For now, we wont provide any prebuilt binaries and instead rely on the fallback mechanism to load the correct module. I will reevaluate this if it causes issues with bundling and look to add proper runtime electron detection.
…o 2.9.7 dep (#14279) We wanted to ensure our nitro rollup plugin fix also properly works for the lowest nuxt version the nuxt SDK supports. This pins nitro to `2.9.7` and nuxt to `3.13.2`.
…an.updateName` on `SentrySpan` (#14251) Fix a "regression" introduced in v8 where we no longer updated the source of the span when calling `span.updateName`. We had this behaviour in our v7 `Transaction` class (IIRC via `transaction.setName(name, source)` but most likely forgot to port it to the `Span` class as the `event.transaction_info.source` field is only relevant for transactions (i.e. root spans in today's SDK lingo).
Change to stop restarting canvas manager web worker when stopping recording.
…14306) This is not really necessary anymore - it only sets this on transaction events, and those get the `transaction` in different places already anyhow. With this, we can also actually remove some other stuff. One method is exported from utils but not otherwise used, we can also drop this in v9. Finally, this was also the only place that used `route` on the request, so we can also get rid of this in `remix`, which is weird anyhow because we set it for errors there but don't even use it for them. --------- Co-authored-by: Luca Forstner <[email protected]>
This PR aims to solve two things: 1. Do not require us to keep a list of test-applications to run on CI. This is prone to be forgotten, and then certain tests do not run on CI and we may not notice (this has happened more than once in the past 😬 ) 2. Do not run E2E test applications that are unrelated to anything that was changed. With this PR, instead of keeping a list of E2E test jobs we want to run manually in the build.yml file, this is now inferred (on CI) by a script based on the nx dependency graph and some config in the test apps package.json. This is how it works: 1. Pick all folders in test-applications, by default all of them should run. --> This will "fix" the problem we had sometimes that we forgot to add test apps to the build.yml so they don't actually run on CI :grimacing: 3. For each test app, look at it's dependencies (and devDependencies), and see if any of them has changed in the current PR (based on nx affected projects). So e.g. if you change something in browser, only E2E test apps that have any dependency that uses browser will run, so e.g. node-express will be skipped. 4. Additionally, you can configure variants in the test apps package.json - instead of defining this in the workflow file, it has to be defined in the test app itself now. 5. Finally, a test app can be marked as optional in the package.json, and can also have optional variants to run. 6. The E2E test builds the required & optional matrix on the fly based on these things. 7. In pushes (=on develop/release branches) we just run everything. 8. If anything inside of the e2e-tests package changed, run everything. --> This could be further optimized to only run changed test apps, but this was the easy solution for now. ## Example test runs * In a PR with no changes related to the E2E tests, nothing is run: https://github.com/getsentry/sentry-javascript/actions/runs/11838604965 * In a CI run for a push, all E2E tests are run: https://github.com/getsentry/sentry-javascript/actions/runs/11835834748 * In a PR with a change in e2e-tests itself, all E2E tests are run: https://github.com/getsentry/sentry-javascript/actions/runs/11836668035 * In a PR with a change in the node package, only related E2E tests are run: https://github.com/getsentry/sentry-javascript/actions/runs/11838274483
Ref #14242 The NestJS SDK is stable enough for us to remove this label. All the breaking changes we do to the SDK are gonna end up in v9.
…s` SDK `init()` (#14321)
v1 of this is deprecated, which you can see in each CI run on develop as of now. Updating this should fix the deprecation. This _should_ not break stuff for us: https://www.npmjs.com/package/@actions/artifact#v2---whats-new
…adcrumbIntegration` and deprecate it (#14334)
This PR cleans up some jest test output: 1. Ensure we do avoid printing out some `console.error` stuff for invalid DSN parsing. 2. Update jest config to avoid printing of these warnings you get all the time in the tests: ``` ts-jest[config] (WARN) message TS151001: If you have issues related to imports, you should consider setting `esModuleInterop` to `true` in your TypeScript configuration file (usually `tsconfig.json`). See https://blogs.msdn.microsoft.com/typescript/2018/01/31/announcing-typescript-2-7/#easier-ecmascript-module-interoperability for more information. ts-jest[config] (WARN) message TS151001: If you have issues related to imports, you should consider setting `esModuleInterop` to `true` in your TypeScript configuration file (usually `tsconfig.json`). See https://blogs.msdn.microsoft.com/typescript/2018/01/31/announcing-typescript-2-7/#easier-ecmascript-module-interoperability for more information. ts-jest[config] (WARN) message TS151001: If you have issues related to imports, you should consider setting `esModuleInterop` to `true` in your TypeScript configuration file (usually `tsconfig.json`). See https://blogs.msdn.microsoft.com/typescript/2018/01/31/announcing-typescript-2-7/#easier-ecmascript-module-interoperability for more information. ts-jest[config] (WARN) message TS151001: If you have issues related to imports, you should consider setting `esModuleInterop` to `true` in your TypeScript configuration file (usually `tsconfig.json`). See https://blogs.msdn.microsoft.com/typescript/2018/01/31/announcing-typescript-2-7/#easier-ecmascript-module-interoperability for more information. ts-jest[config] (WARN) message TS151001: If you have issues related to imports, you should consider setting `esModuleInterop` to `true` in your TypeScript configuration file (usually `tsconfig.json`). See https://blogs.msdn.microsoft.com/typescript/2018/01/31/announcing-typescript-2-7/#easier-ecmascript-module-interoperability for more information. ts-jest[config] (WARN) message TS151001: If you have issues related to imports, you should consider setting `esModuleInterop` to `true` in your TypeScript configuration file (usually `tsconfig.json`). See https://blogs.msdn.microsoft.com/typescript/2018/01/31/announcing-typescript-2-7/#easier-ecmascript-module-interoperability for more information. ts-jest[config] (WARN) message TS151001: If you have issues related to imports, you should consider setting `esModuleInterop` to `true` in your TypeScript configuration file (usually `tsconfig.json`). See https://blogs.msdn.microsoft.com/typescript/2018/01/31/announcing-typescript-2-7/#easier-ecmascript-module-interoperability for more information. ts-jest[config] (WARN) message TS151001: If you have issues related to imports, you should consider setting `esModuleInterop` to `true` in your TypeScript configuration file (usually `tsconfig.json`). See https://blogs.msdn.microsoft.com/typescript/2018/01/31/announcing-typescript-2-7/#easier-ecmascript-module-interoperability for more information. ts-jest[config] (WARN) message TS151001: If you have issues related to imports, you should consider setting `esModuleInterop` to `true` in your TypeScript configuration file (usually `tsconfig.json`). See https://blogs.msdn.microsoft.com/typescript/2018/01/31/announcing-typescript-2-7/#easier-ecmascript-module-interoperability for more information. ```
size-limit report 📦
|
andreiborza
approved these changes
Nov 19, 2024
lforst
force-pushed
the
prepare-release/8.39.0
branch
from
82cac69 to
8659f8d
Compare
andreiborza
approved these changes
Nov 19, 2024
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.
No description provided.