diff --git a/.github/actions/install-dependencies/action.yml b/.github/actions/install-dependencies/action.yml index 27971a484f63..2a4b331fd3f6 100644 --- a/.github/actions/install-dependencies/action.yml +++ b/.github/actions/install-dependencies/action.yml @@ -11,6 +11,7 @@ runs: echo 'paths<> "$GITHUB_OUTPUT" shell: bash diff --git a/.github/actions/install-playwright/action.yml b/.github/actions/install-playwright/action.yml index bd84a13e9107..378e77666093 100644 --- a/.github/actions/install-playwright/action.yml +++ b/.github/actions/install-playwright/action.yml @@ -34,6 +34,13 @@ runs: playwright apt + # `--with-deps` names packages that the runner image already has (glib, mesa, nss, ...), and `apt-get install` would + # upgrade them to the latest Ubuntu update. That is half of the download on a cold cache, and the image versions work. + - name: Keep installed apt packages + if: steps.playwright-version.outputs.version != '' + run: echo 'APT::Get::Upgrade "false";' | sudo tee /etc/apt/apt.conf.d/99-no-upgrade > /dev/null + shell: bash + # Downloads only the browsers and apt packages that are not on the cache volume yet. `npx` runs outside the repo, # so it can't pick up a `playwright` that a parallel `pnpm install` is still linking. - name: Install Playwright diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8e65611d4c50..d59dc8f590a4 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -343,6 +343,14 @@ jobs: run: pnpm run lint:es-compatibility - name: Lint types run: pnpm run lint:types + - name: Check for circular dependencies + run: pnpm run circularDepCheck + - name: Check for dts files that reference stuff in the temporary build folder + run: | + if grep -r --include "*.d.ts" --exclude-dir ".nx" 'import("@sentry(-internal)?/[^/]*/build' .; then + echo "Found illegal TypeScript import statement." + exit 1 + fi - name: Check Playwright versions run: | version=$(node -p "require('./dev-packages/test-utils/package.json').devDependencies['@playwright/test']") @@ -406,27 +414,6 @@ jobs: - name: Check file formatting run: pnpm run format:check - job_circular_dep_check: - name: Circular Dependency Check - needs: [job_get_metadata, job_build] - timeout-minutes: 10 - runs-on: - namespace-profile-sentry-javascript;overrides.cache-tag=sentry-javascript${{ github.event_name == 'pull_request' - && '-pr' || '' }} - steps: - - name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }}) - uses: actions/checkout@v7 - with: - ref: ${{ env.HEAD_COMMIT }} - - name: Set up Node - uses: actions/setup-node@v7 - with: - node-version-file: 'package.json' - - name: Restore caches - uses: ./.github/actions/restore-cache - - name: Run madge - run: pnpm run circularDepCheck - job_artifacts: name: Upload Artifacts needs: [job_get_metadata, job_build, job_build_layer, job_build_bundles, job_build_tarballs] @@ -772,35 +759,8 @@ jobs: overwrite: true retention-days: 7 - job_check_for_faulty_dts: - name: Check for faulty .d.ts files - needs: [job_get_metadata, job_build] - runs-on: - namespace-profile-sentry-javascript;overrides.cache-tag=sentry-javascript${{ github.event_name == 'pull_request' - && '-pr' || '' }} - timeout-minutes: 5 - steps: - - name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }}) - uses: actions/checkout@v7 - with: - ref: ${{ env.HEAD_COMMIT }} - - name: Set up Node - uses: actions/setup-node@v7 - with: - node-version-file: 'package.json' - - name: Restore caches - uses: ./.github/actions/restore-cache - - name: Check for dts files that reference stuff in the temporary build folder - run: | - if grep -r --include "*.d.ts" --exclude-dir ".nx" 'import("@sentry(-internal)?/[^/]*/build' .; then - echo "Found illegal TypeScript import statement." - exit 1 - fi - job_node_integration_tests: - name: - Node (${{ matrix.node }})${{ (matrix.typescript && format(' (TS {0})', matrix.typescript)) || '' }} Integration - Tests (${{ matrix.shard }}/3) + name: Node (${{ matrix.node }}) Integration Tests (${{ matrix.shard }}/3) needs: [job_get_metadata, job_build] if: needs.job_build.outputs.changed_node_integration == 'true' || github.event_name != 'pull_request' runs-on: @@ -812,19 +772,6 @@ jobs: matrix: node: [20.19, 22, 24, 26] shard: [1, 2, 3] - typescript: - - false - include: - # Only check typescript for latest version (to streamline CI) - - node: 24 - typescript: '5.0' - shard: 1 - - node: 24 - typescript: '5.0' - shard: 2 - - node: 24 - typescript: '5.0' - shard: 3 steps: - name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }}) uses: actions/checkout@v7 @@ -850,11 +797,6 @@ jobs: - name: Restore caches uses: ./.github/actions/restore-cache - - name: Overwrite typescript version - if: matrix.typescript == '5.0' - run: node ./scripts/use-ts-5_0.js - working-directory: dev-packages/node-integration-tests - - name: Run integration tests working-directory: dev-packages/node-integration-tests run: pnpm run test --shard=${{ matrix.shard }}/3 @@ -1116,9 +1058,10 @@ jobs: runtime: ${{ matrix.runtime }} deno-version: ${{ matrix.deno-version }} - # Installs Playwright while the app builds. Keep it right before the build: the runner sets up a background - # composite action's steps from the job's shared `github` context, which fails with "Collection was modified" if - # a foreground step starts at that moment. Starting next to `download-artifact` also made it fail to read the event file. + # Installs Playwright while the app installs and builds. Keep it right before the install: the runner sets up a + # background composite action's steps from the job's shared `github` context, which fails with "Collection was + # modified" if a foreground step starts at that moment. Starting next to `download-artifact` also made it fail to + # read the event file. - name: Install Playwright id: install-playwright background: true @@ -1127,6 +1070,23 @@ jobs: browsers: chromium cwd: dev-packages/e2e-tests/test-applications/${{ matrix.test-application }} + # Every build command runs `pnpm install`, so installing first makes that a no-op. Some downloads from the npm + # registry crawl at tens of kB/s for the whole 7 minutes of the build step, and pnpm does not give up on a slow + # download. Another attempt reuses what is already in the store and opens new connections. + # The hoisted variant installs with another node linker, so installing first would only redo the install. + - name: Install E2E app dependencies + if: ${{ !contains(matrix.build-command, 'hoisted') }} + working-directory: ${{ runner.temp }}/test-application + timeout-minutes: 7 + run: | + for attempt in 1 2; do + timeout 150 pnpm install && exit 0 + echo "::warning::pnpm install attempt $attempt failed or did not finish in 150 s, retrying" + done + pnpm install + env: + SENTRY_E2E_WORKSPACE_ROOT: ${{ github.workspace }} + - name: Build E2E app working-directory: ${{ runner.temp }}/test-application timeout-minutes: 7 @@ -1217,7 +1177,6 @@ jobs: job_lint, job_check_lockfile, job_check_format, - job_circular_dep_check, job_size_check, ] # Always run this, even if a dependent job failed diff --git a/.github/workflows/enforce-license-compliance.yml b/.github/workflows/enforce-license-compliance.yml index e748668b92db..5d44ccb3497e 100644 --- a/.github/workflows/enforce-license-compliance.yml +++ b/.github/workflows/enforce-license-compliance.yml @@ -17,6 +17,12 @@ on: - v9 - v8 +# Cancel in progress workflows on pull_requests. +# https://docs.github.com/en/actions/using-jobs/using-concurrency#example-using-a-fallback-value +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + jobs: enforce-license-compliance: runs-on: namespace-profile-sentry-javascript diff --git a/dev-packages/e2e-tests/test-applications/aws-serverless-layer/tests/layer.test.ts b/dev-packages/e2e-tests/test-applications/aws-serverless-layer/tests/layer.test.ts index 538b533967e0..8e47ac4255a4 100644 --- a/dev-packages/e2e-tests/test-applications/aws-serverless-layer/tests/layer.test.ts +++ b/dev-packages/e2e-tests/test-applications/aws-serverless-layer/tests/layer.test.ts @@ -337,7 +337,8 @@ test.describe('Lambda layer', () => { const transactionEvent = await transactionEventPromise; expect(transactionEvent.contexts?.trace?.status).toBe('ok'); - expect(transactionEvent.timestamp! - transactionEvent.start_timestamp!).toBeGreaterThanOrEqual(0.2); + // Node timers can fire up to a millisecond early, so the 200 ms callback delay can measure as 199.x ms + expect(transactionEvent.timestamp! - transactionEvent.start_timestamp!).toBeGreaterThanOrEqual(0.19); }); test('sets error status when a callback-style handler calls back with an error after it returned', async ({ @@ -360,7 +361,7 @@ test.describe('Lambda layer', () => { const [errorEvent, transactionEvent] = await Promise.all([errorEventPromise, transactionEventPromise]); expect(transactionEvent.contexts?.trace?.status).toBe('internal_error'); - expect(transactionEvent.timestamp! - transactionEvent.start_timestamp!).toBeGreaterThanOrEqual(0.2); + expect(transactionEvent.timestamp! - transactionEvent.start_timestamp!).toBeGreaterThanOrEqual(0.19); expect(errorEvent.contexts?.trace?.trace_id).toBe(transactionEvent.contexts?.trace?.trace_id); }); diff --git a/dev-packages/node-integration-tests/scripts/use-ts-5_0.js b/dev-packages/node-integration-tests/scripts/use-ts-5_0.js deleted file mode 100644 index 281bb4e9dcb7..000000000000 --- a/dev-packages/node-integration-tests/scripts/use-ts-5_0.js +++ /dev/null @@ -1,16 +0,0 @@ -/* eslint-disable no-console */ -const { execSync } = require('child_process'); -const { join } = require('path'); - -const cwd = join(__dirname, '../../..'); - -const tsVersion = '5.0.4'; - -console.log(`Installing typescript@${tsVersion}, and @types/node@18...`); - -execSync(`pnpm add --save-dev --workspace-root typescript@${tsVersion} @types/node@^18`, { - stdio: 'inherit', - cwd, -}); - -console.log('TypeScript version updated successfully.'); diff --git a/packages/replay-internal/src/coreHandlers/handleClick.ts b/packages/replay-internal/src/coreHandlers/handleClick.ts index f4576c3a7ca7..c61278494646 100644 --- a/packages/replay-internal/src/coreHandlers/handleClick.ts +++ b/packages/replay-internal/src/coreHandlers/handleClick.ts @@ -183,10 +183,10 @@ export class ClickDetector implements ReplayClickDetector { const now = nowInSeconds(); this._clicks.forEach(click => { - if (!click.mutationAfter && this._lastMutation) { + if (click.mutationAfter === undefined && this._lastMutation) { click.mutationAfter = click.timestamp <= this._lastMutation ? this._lastMutation - click.timestamp : undefined; } - if (!click.scrollAfter && this._lastScroll) { + if (click.scrollAfter === undefined && this._lastScroll) { click.scrollAfter = click.timestamp <= this._lastScroll ? this._lastScroll - click.timestamp : undefined; } @@ -215,8 +215,9 @@ export class ClickDetector implements ReplayClickDetector { /** Generate matching breadcrumb(s) for the click. */ private _generateBreadcrumbs(click: Click): void { const replay = this._replay; - const hadScroll = click.scrollAfter && click.scrollAfter <= this._scrollTimeout; - const hadMutation = click.mutationAfter && click.mutationAfter <= this._threshold; + // A mutation or scroll in the same millisecond as the click gives `0`, which still counts + const hadScroll = click.scrollAfter !== undefined && click.scrollAfter <= this._scrollTimeout; + const hadMutation = click.mutationAfter !== undefined && click.mutationAfter <= this._threshold; const isSlowClick = !hadScroll && !hadMutation; const { clickCount, clickBreadcrumb } = click; diff --git a/packages/replay-internal/test/unit/coreHandlers/handleClick.test.ts b/packages/replay-internal/test/unit/coreHandlers/handleClick.test.ts index 66beddf6ab19..7a51c5ca3e5b 100644 --- a/packages/replay-internal/test/unit/coreHandlers/handleClick.test.ts +++ b/packages/replay-internal/test/unit/coreHandlers/handleClick.test.ts @@ -321,6 +321,22 @@ describe('Unit | coreHandlers | handleClick', () => { expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); }); + test('it does not consider clicks with a mutation in the same millisecond as slow click', async () => { + const breadcrumb: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 1, + }, + }; + const node = document.createElement('button'); + detector.handleClick(breadcrumb, node); + detector.registerMutation(BASE_TIMESTAMP); + + vi.advanceTimersByTime(3_500); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + }); + test('it considers clicks with mutation after threshold as slow click', async () => { const breadcrumb: Breadcrumb = { timestamp: BASE_TIMESTAMP / 1000, @@ -429,6 +445,22 @@ describe('Unit | coreHandlers | handleClick', () => { ); }); + test('it does not consider clicks with a scroll in the same millisecond as slow click', async () => { + const breadcrumb: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 1, + }, + }; + const node = document.createElement('button'); + detector.handleClick(breadcrumb, node); + detector.registerScroll(BASE_TIMESTAMP); + + vi.advanceTimersByTime(3_500); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + }); + test('it does not consider clicks with scroll before threshold as slow click', async () => { const breadcrumb: Breadcrumb = { timestamp: BASE_TIMESTAMP / 1000,