ref(node): Allow node stack parser to work in browser context #5134
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
Removes all references to `@sentry/apm`, a deprecated package we do not use anymore.
Removes the deprecated `API` class.
…entry#4849) Remove deprecated methods `startSpan` and `child`. These deprecated methods were removed in favour of `span.startChild`.
Removes `getActiveDomain` function and corresponding type.
The `user` dsn component was renamed to `publicKey`. This PR removes that from the dsn field.
Increases the creation timeout of `MongoMemoryServer` to temporarily fix Node integration test flakiness
This PR removes the previously deprecated `frameContextLines` from `NodeOptions`.
These exports were historically used in `@sentry/electron`, but are no longer being used by the Electron SDK or the React Native SDK, so they can be removed.
This updates the SDK to use Typescript 3.8.3, in order to be able to use type-only imports and exports[1]. (These are needed for us to turn on `isolatedModules`, which is in turn is needed for us to switch our build tool away from `tsc`[2], since no other tool understands the relationship between files.)
As a result of this change, a few of the browser integration tests needed to be fixed so that all promises were explicitly awaited, a point about which the linter in 3.8 complains.
This is a breaking change for anyone using TS 3.7.x (there's no one using TS < 3.7.x, since that's our current minimum). That said, though there are plenty of public projects on GH using TS 3.7 and `@sentry/xyz`, if you restrict it to projects using TS 3.7 and `@sentry/xyz` 6.x, all you get are forks of this very repo. Granted, this isn't every project ever, but it's likely decently representative of the fact that if you've upgraded our SDK, you've almost certainly upgraded TS as well. We're going to wait until v7 to release this change in any case, but that's an indication that it won't affect many people when we do. (See this commit's PR for links to searches demonstrating the points.)
Note: Originally there was some thought of going farther, into TS 4.x, but we decided that for the foreseeable future, we're going to stick with 3.8.3. Though moving up to 4.x doesn't seem like it would affect many customers (repeating the same TS/sentry 6.x crossover search with TS 3.8 and 3.9 reveals a total of 5 projects), it does have the known side effect of replacing export statements which, after compilation, currently look like
`exports.SdkInfo = types_1.SdkInfo;`
with ones that look like
`Object.defineProperty(exports, "SdkInfo", { enumerable: true, get: function () { return types_1.SdkInfo; } });`.
(For those of you following along at home, that's a 3x character increase.) Though we might be able to engineer around this in order to avoid the inevitable substantial bundle size hit, attempting to do so is only worth it if there are features from 4.x that we desperately need. Given that we've agreed that right now there aren't, we'll stick with 3.8.3.
[1] https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#type-only-imports-and-export
[2] https://www.typescriptlang.org/tsconfig#isolatedModules
rename the dist directories in all build dirs to cjs. Hence, also the tarballs' structure changes which is why this PR also introduces a breaking change. Additional change: cleanup `yarn clean` commands by removing no longer existing directories
…try#4906) In getsentry#4842, a `flags.ts` file was added to each package to fix logger treeshaking when using webpack. Because of the way that webpack works, this file has to exist separately in each individual package, including in `@sentry/integrations`. It is not itself an integration, though, so we shouldn't be building a separate CDN bundle for it (let alone six versions of one). This fixes the build script so that we're no longer doing that.
Now that we have files like `rollup.config.js` and `jest.config.js` at the package root level, it's helpful if running `yarn clean` doesn't delete them. This fixes that problem, and also fixes a spot where the now-defunct package-top-level `esm` directory was still included in a `clean` script.
…ry#4905) The `.gitignore` file in the `utils` package isn't ignoring any current files which the main `.gitignore` (the one at the root level of the repo) isn't already ignoring, as evidenced by the fact that deleting the former doesn't cause anything to new to come to git's attention. What it will ignore, however, is future `.js` files (such as `rollup.config.js` and `jest.config.js`) which we don't want ignored (and which aren't ignored in any other package). This therefore removes the `.gitignore` in `utils`, in order to prevent that future problem.
Currently, we configure jest using a mix of `package.json` entries and `jest.config.js` files, and there's a great deal of repetition between configs. To make things DRYer and easier to update (as will happen in future PRs), this aims to fix that by creating a centralized jest config from which all others can inherit. To facilitate this inheritance, all config has been removed from `package.json` files and moved into `jest.config.js` files. This change was done in a few distinct stages: - Extracting the config which was identical across many packages into a central config file, and fixing a mistake they all contained, namely that they were using the regular tsconfig file rather than the test-specific one. - In the packages which were using exactly that config, creating a new `jest.config.js` file and inheriting directly from the central jest config with no changes. - In the packages whose config varied only slightly from the boilerplate config, creating a new `jest.config.js` file and inheriting from the central file with small changes. This also required adding `.tsx` files to the central config. - In the browser package, moving the existing `jest.config.js` for the unit tests to the repo root level and refactoring it to inherit from the central file. This also required specifying a coverage directory in the central config and modifying the browser package's yarn test commands. - In the node integration test package, refactoring the existing `jest.config.js` to inherit from the central file. This also required creating a test-specific tsconfig to match the one in other packages. - Finally, making a small optimization (narrowing the scope of where to look for tests) in the now universally-used central config.
Documents: - ES6 for CJS files - Dropping Support for Node v6 - Removing Platform Integrations - New npm package structure - Deleting deprecations
…entry#4914) Our old browser integration tests are difficult to debug. This adds a file of (hard-won) debugging tips to the test directory, to hopefully make it easier on the next person. While it's true that it contains a lot of very specific references (to functions, files, etc) and is therefore much more susceptible to becoming out of date, these tests aren't something we change often, and the consequences of such staleness are small.
…y#4885) Removes usage of deprecated `event.stacktrace` and updated the event interface to remove the deprecated property.
This updates `jest`, `ts-jest`, and `jest-environment-node` to the latest versions, in order to facilitate code transformations during `ts-jest`'s on-the-fly compilation that will become necessary once we move to ES6. (More detail on this to come in the PR which actually introduces said transformation, but TL;DR the way we use and extend `global` is fine if it's a `var` (which it is in ES5 Land) but less fine if it's a `const` (which it becomes under ES6), and we need to fix that for tests to run.) It also updates `jsdom`. Together these updates meant that a larger number of packages needed to be downgraded in order for tests to run in node 8 and 10. This therefore also reworks the test script a bit to account for those changes. Finally, this removes the test environment from our main jest config, as its value has become the default in latest version of jest.
…etsentry#5093) * Removed zipping from build script * Assemble Lambda Layer zip using GitHub action. Co-authored-by: Lukas Stracke <lukas@stracke.co.at>
…tsentry#5100) * Fix for properly handling SDK version in GitHub actions.
This parallelizes our repo-level build yarn scripts where possible, in order to make them run a bit faster. In the end, the time savings isn't enormous, but still worthwhile: in a series of 10 time trials on GHA, the old build averaged ~9 minutes, while the new one averaged a bit over 8 minutes. (The time savings isn't that dramatic because the current lack of parallelity at the repo level isn't the biggest driver of our slow build speed; that would be the lack of parallelity at the individual package level, a problem which will be tackled in future PRs. Doing this now will let us take full advantage of that future work.) The over-arching goal here was to make as few tasks as possible have to wait on other tasks. Simply because of computing power, it isn't realistic to run _all_ tasks, for the entire repo, simultaneously. But the closer we can get to maxing out the theoretical potential for infinite parallelization, the better off we'll be, because then the only constraint does become the build runner. In order to accomplish this maximum-possible parallelization, a few things had to be considered: - Is there any interdependency between packages which means that a particular task can't be done in all packages simultaneously? - Is there any interdependency between tasks within a package that means that they can't be done simultaneously? - How do we make sure that at both the repo and package level, `yarn build:dev` builds only what's needed for local development and testing and `yarn build` builds everything? - Is there a way to organize things such that it works for every package, even ones with non-standard build setups? After investigation, it turned out that the key constraints were: - Types can't be built entirely in parallel across packages, because `tsc` checks that imports are being used correctly, which it can't do if types for the packages exporting those imports aren't yet themselves built. - Rollup and bundle builds can happen in parallel across packages, because each individual package's tasks are independent of the same tasks in other packages. Further, type-, rollup-, and bundle-building as overall process are also independent (so, for example, rollup builds don't have to wait on type builds). - Some packages have build tasks in addition to types, rollup, and bundles, and in some cases those tasks can't happen until after the rest of the build completes. - Some packages (angular and ember) have their own build commands, and don't have types, rollup, or bundle builds. To solve these constraints, the build system now follows these principles: - Every build task, in every package, is now represented in its package in exactly one of four scripts: `yarn build:types`, `yarn build:rollup`, `yarn build:bundle`, and a new `yarn build:extras`. Tasks can be parts of other package-level scripts (types and rollup builds together forming each package's `yarn build:dev`, for example), but as long as every task is in one of the canonical four, running those canonical scripts in every package that has them means we're guaranteed not to miss anything. - Types are build using lerna's `--stream` option, now with no limit on concurrency, in order to parallelize where possible without breaking dependency ordering. - Types are built independently of the other three kinds of tasks, since everything else _can_ be fully parallelized. This means not using package-level `yarn build` or `yarn build:dev` commands in the repo-level build, since they tie other build tasks to types build tasks. - To make things as easy as possible to reason about, not just types, but all four task kinds are therefore addressed separately by the repo-level build. - Angular and ember's build tasks are each aliased to a `yarn build:extras` script, so as not to be left out. - Because some "extras" tasks can't be done until types, rollup, and/or bundle tasks are done, make it so all "extras" tasks are held until the other three kinds have finished. This does push a few later than need be, but it's worth it in order to standardize the logic. - All of these principles are duplicated at the package level. For the visual learners among us, there is diagram illustrating this in the PR description. Oh, and while I was in there, I got rid of `yarn build:dev:filter`. It doesn't fit the new system, and no one uses it (not even me, and I made it up).
This splits the repo-level `yarn clean` command, which currently leaves the repo in a broken state (build artifacts deleted, and unable to be rebuilt because package-level `node_modules` folders - though not the top `node_modules` folder - have also been deleted), into a number of new, more focused commands. In the `clean:deps` command, it now deletes _all_ `node_modules` folders (repo- and package-level) and reinstalls dependencies, so that the repo is no longer broken after running it. The new commands: ``` // Meant to be a useful default for day-to-day use "clean": "run-p clean:build clean:caches" // Runs all package-level clean commands, which delete build and testing artifacts "clean:build": "lerna run --parallel clean" // TODO: Are there other caches we should add here? "clean:caches": "yarn rimraf eslintcache && yarn jest --clearCache" // Nuke all node modules and reinstall dependencies "clean:deps": "lerna clean --yes && rm -rf node_modules && yarn" // insert "Clean all the things!" meme here "clean:all": "run-p clean:build clean:caches clean:deps" ```
This fixes a spot where we're importing types only to _say_ that we're importing types only, in order to get rid of a warning in build. (If we don't say so, it gets mad because we only have `@types/express`, not `express`, as a listed dependency.)
This removes `.gitignore` files containing redundant entries (ones covered by the main `.gitignore`) and organizes the main `.gitignore` just a bit. It also removes the old build directories, `cjs` and `esm`, from the main `.gitignore`.
Releasing a patch to get the express import change, which turns to to fix a bug with the structure of the build artifacts for serverless.
Currently, as part of caching our built files in GHA, we cache any directory named `build`, `cjs`, or `esm`, no matter how deeply nested. As a result, we end up caching files from any number of node modules which happen to contain directories of the same name(s). Since we have a separate dependency cache, this is redundant and only serves to slow down our CI checks. This fixes that problem by narrowing the scope to the `build` folder at the top level of each package, so that now we upload ~2700 files rather than ~4000. Two legitimate files which would otherwise be excluded as a result of this change have also been added to the cache.
This does some clean up work on our two main tsconfig files, in `@sentry/typescript` and at the root level of our repo. Included changes: - Removed `pretty` - what we had is the default - Removed `noImplicitAny` and `noImplicitThis` - implied by `strict: true` - Turned off emitting types files, as we now do this separately - Moved `noErrorTruncation` up to the typescript package, so that anyone (is there anyone besides us?) using it gets the benefit of actually being able to see full errors
…y#5112) The `removeESLintCommentsPlugin` rollup plugin we use during build strips all comments beginning `eslint-disable`, but misses comments beginning `eslint-enable`. This makes the regex used in that plugin more general, so that it catches all comments beginning with `eslint-`.
When the tsconfig option`skipLibCheck`[1] is false (as it is by default), TS typechecks not only your code, and not only the code you use from dependencies, but the entire codebase of all of your dependencies. Not only does this seem like overkill, in our case it's actually largely redundant, because our packages have very few dependencies other than each other. Turning this off speeds up the build and prevents us from, for example, typechecking the entire `utils` package 15 times (once for its own sake, and once for every one of the 14 packages which depend on it). [1] https://www.typescriptlang.org/tsconfig#skipLibCheck
This switches the building of our es6 CDN bundles to use sucrase instead of `tsc` (or, more accurately, to use the sucrase rollup plugin rather than the typescript rollup plugin), in order both to build more quickly (in a time trial of multiple runs on GHA, this change brings the average `yarn build` run down from ~8 minutes to ~4 minutes) and to ensure that we have exact code parity between our CDN bundles and our npm packages. Because sucrase doesn't down-compile the way `tsc` will, the building of the es5 bundles hasn't been changed, and that build still uses the typescript plugin.
When running tests, React will throw a warning unless you surround state changes with `act(() => <state change>)`[1]. This does that everywhere necessary to get rid of the warning. [1] https://reactjs.org/docs/test-utils.html#act
This patch introduced a basic data structure for baggage, as well as controls to create, manipulate and serialize the baggage data structure. The baggage data structure represents key,value pairs based on the baggage spec: https://www.w3.org/TR/baggage It is expected that users interact with baggage using the helpers methods: `createBaggage`, `getBaggageValue`, and `setBaggageValue`. Internally, the baggage data structure is a tuple of length 2, separating baggage values based on if they are related to Sentry or not. If the baggage values are set/used by sentry, they will be stored in an object to be easily accessed. If they are not, they are kept as a string to be only accessed when serialized at baggage propagation time. As a next step, let's add the baggage values to the envelope header so they can be processed and used by relay - this will allow us to do some early validations of our assumptions.
This moves, without making any changes, the aws lambda build job to live with the other build job in our CI workflow. (I'll admit that it's not totally obvious, but there is in fact a vague organizing principle to that doc.) Extracted from another PR to make it easier to see, in that PR, what actual changes are being made.
…5119) In getsentry#5094, a change was made to parallelize our repo-level build commands as much as possible. In that PR, it was stated that `build:bundle` could run independent of, and therefore in parallel with, the types and rollup builds, and at the time that was true. When TS (through rollup) builds a bundle, it creates a giant AST out of the bundled package and all of its monorepo dependencies, based on the original source code, then transpiles the whole thing - no prework needed. But in getsentry#5111 we switched our ES6 bundles to use sucrase for transpilation (still through rollup), and that changed things. Sucrase (along with every other non-TS transpilation tool) only considers files one by one, not as part of a larger whole, and won't reach across package boundaries, even within the monorepo. As a result, rollup needs all dependencies to already exist in transpiled form, since sucrase doesn't touch them, which becomes a problem if both processes are happening at once. (_But how has CI even been passing since that second PR, then?_, you may ask. The answer is, sucrase is very fast, and lerna can only start so many things at once. It ends up being a race condition between sucrase finishing with the dependencies and lerna kicking off the bundle builds, and almost all the time, sucrase wins. And the other situations in which this is broken all involve using something other than the top-level `build` script to create bundles in a repo with no existing build artifacts, and CI just never does that.) So TL;DR, we need to have already transpiled a packages's monorepo dependencies before that package can be turned into a bundle. For `build:bundle` at both the repo and package level, and for `build` at the package level, this means that if they're not there, we have to build them. For `build` at the repo level, where transpilation of all packages does in fact already happen, we have two options: 1) Push bundle builds to happen alongside `build:extras`, after rollup builds have finished. 2) Mimic what happens when the race condition is successful, but in a way that isn't flaky. In other words, continue to run `build:bundle` in parallel with the types and npm package builds, but guarantee that the needed dependencies have finished building themselves before starting the bundle build. Of the two options, the first is certainly simpler, but it also forces the two longest parts of the build (bundle and types builds) to be sequential, which is the exact _opposite_ of what we want, given that the goal all along has been to make the builds noticeably faster. Choosing the second solution also gives us an excuse to add the extra few lines of code needed to fix the repo-level-build:bundle/package-level-build/package-level-build:bundle problem, which we otherwise probably wouldn't fix. This implements that second strategy, by polling at 5 second intervals for the existence of the needed files, for up to 60 seconds, before beginning the bundle builds. (In practice, it fairly reliably seems to take two retries, or ten seconds, before the bundle build can begin.) It also handles the other three situations above, by building the missing files when necessary. Finally, it fixes a type import to rely on the main types package, rather than a built version of it. This prevents our ES5 bundles (which still use TS for transpilation, in order to also take advantage of its ability to down-compile) from running to the same problem as the sucrase bundles are having.
…es to TL;DR section (getsentry#5125) add a point to the TL;DR section stating that other Sentry packages like `@sentry/tracing` should be upgraded alongside the used Sentry SDK
This patch Introduces the first step of getting Dynamic Sampling working with the JS SDKs. It adds a very simple approach of attaching baggage to the envelope header of a transaction's event envelope. It does not take immutability of baggage in multiple transactions per trace scenarios into account. This is something we have to address when we're going to add the baggage header to outgoing requests. However, the simple approach should be enough to send baggage information to relay to get DS working on individual transactions.
…ion. (getsentry#5126) If the serverless SDK is running in an AWS Lambda Function it should not send data directly to sentry.io but to the relay that is running in the AWS Lambda Extension (which is running on localhost:3000) This PR parses the DSN and changes the host and port to point to localhost:3000.
- Adds `Attachment` interface and extends `AttachmentItemHeaders` with required fields - Adds `addAttachment`, `getAttachments` and `clearAttachments` to `Scope` - Adds `attachments: Attachment[]` to `EventHint` - In the private capture/process/prepare/send methods in the `BaseClient`, the `EventHint` argument has moved as it's no longer optional - We can't mutate the hint if it's undefined! - Copies attachments from the scope to the hint in `_prepareEvent` - Adds optional `textEncoder` to `InternalBaseTransportOptions` and overrides this in the node client to support node 8-10 - Manually pass `new TextEncoder()` in many of the tests so they pass on node.js 8-10 - Adds binary serialisation support for envelopes - `serializeEnvelope` returns `string | Uint8Array` which all transports supported without modification - Defaults to concatenating strings when no attachments are found. String concatenation is about 10x faster than the binary serialisation - Rewrites `parseEnvelope` in `testutils.ts` so that it can parse binary envelopes
timfish
changed the title
|
Should have been on v7! |
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.
Depending on configuration, Electron may have mixed Chrome/node.js stack traces and to decode these we need to use the node stack parser in the Electron renderers. Because the default Electron configuration does not have mixed Chrome/node.js context, for maximum compatibility, our renderer code must always be browser/bundler compatible which poses some issues when using the node.js stack parser.
The only node.js specific code in the stack parser fetches the
moduleusing node.js features (require.main.filename,global.process.cwd()). This PR makes thegetModulefunction an optional parameter and reorganises the file structure so we can import the required code without hitting anything node specific which will upset bundlers: