From b8ae447b0fdc12553b4eacd26595e4054ffcf415 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Sat, 17 Dec 2022 17:18:49 -0800 Subject: [PATCH] Fix dev session stopped handling (#44112) This ensures we properly handle the dev session stopped event now that the dev server runs in a different worker so the telemetry globals are no longer available by default in the main process. No additional tests have been added as the existing test caught this. Fixes: https://github.com/vercel/next.js/actions/runs/3715725536/jobs/6301287357 x-ref: https://github.com/vercel/next.js/pull/43958 ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) --- packages/next/cli/next-dev.ts | 73 ++++++++++++------- packages/next/server/dev/next-dev-server.ts | 10 ++- test/integration/telemetry/test/index.test.js | 6 +- 3 files changed, 55 insertions(+), 34 deletions(-) diff --git a/packages/next/cli/next-dev.ts b/packages/next/cli/next-dev.ts index 713026254ee2cd0..420eeab196b0e8a 100755 --- a/packages/next/cli/next-dev.ts +++ b/packages/next/cli/next-dev.ts @@ -8,40 +8,64 @@ import { startedDevelopmentServer } from '../build/output' import { cliCommand } from '../lib/commands' import isError from '../lib/is-error' import { getProjectDir } from '../lib/get-project-dir' -import { CONFIG_FILES } from '../shared/lib/constants' +import { CONFIG_FILES, PHASE_DEVELOPMENT_SERVER } from '../shared/lib/constants' import path from 'path' import type { NextConfig } from '../types' import type { NextConfigComplete } from '../server/config-shared' import { traceGlobals } from '../trace/shared' import cluster from 'cluster' +import { Telemetry } from '../telemetry/storage' +import loadConfig from '../server/config' +import { findPagesDir } from '../lib/find-pages-dir' let isTurboSession = false let sessionStopHandled = false let sessionStarted = Date.now() +let dir: string const handleSessionStop = async () => { if (sessionStopHandled) return sessionStopHandled = true - const { eventCliSession } = - require('../telemetry/events/session-stopped') as typeof import('../telemetry/events/session-stopped') - const telemetry = traceGlobals.get('telemetry') as InstanceType< - typeof import('../telemetry/storage').Telemetry - > - if (!telemetry) { - process.exit(0) - } + try { + const { eventCliSession } = + require('../telemetry/events/session-stopped') as typeof import('../telemetry/events/session-stopped') - telemetry.record( - eventCliSession({ - cliCommand: 'dev', - turboFlag: isTurboSession, - durationMilliseconds: Date.now() - sessionStarted, - pagesDir: !!traceGlobals.get('pagesDir'), - appDir: !!traceGlobals.get('appDir'), - }) - ) - await telemetry.flush() + const config = await loadConfig(PHASE_DEVELOPMENT_SERVER, dir) + + let telemetry = + (traceGlobals.get('telemetry') as InstanceType< + typeof import('../telemetry/storage').Telemetry + >) || + new Telemetry({ + distDir: path.join(dir, config.distDir), + }) + + let appDir: boolean = !!traceGlobals.get('pagesDir') + let pagesDir: boolean = !!traceGlobals.get('appDir') + + if ( + typeof traceGlobals.get('pagesDir') === 'undefined' || + typeof traceGlobals.get('appDir') === 'undefined' + ) { + const pagesResult = await findPagesDir(dir, !!config.experimental.appDir) + appDir = !!pagesResult.appDir + pagesDir = !!pagesResult.pagesDir + } + + telemetry.record( + eventCliSession({ + cliCommand: 'dev', + turboFlag: isTurboSession, + durationMilliseconds: Date.now() - sessionStarted, + pagesDir, + appDir, + }) + ) + await telemetry.flush() + } catch (err) { + console.error(err) + } process.exit(0) } @@ -95,7 +119,7 @@ const nextDev: cliCommand = async (argv) => { process.exit(0) } - const dir = getProjectDir(args._[0]) + dir = getProjectDir(args._[0]) // Check if pages dir exists and warn if not if (!existsSync(dir)) { @@ -147,10 +171,6 @@ const nextDev: cliCommand = async (argv) => { require('../build/webpack-config') as typeof import('../build/webpack-config') const { defaultConfig } = require('../server/config-shared') as typeof import('../server/config-shared') - const { default: loadConfig } = - require('../server/config') as typeof import('../server/config') - const { PHASE_DEVELOPMENT_SERVER } = - require('../shared/lib/constants') as typeof import('../shared/lib/constants') const chalk = require('next/dist/compiled/chalk') as typeof import('next/dist/compiled/chalk') const { interopDefault } = @@ -353,11 +373,8 @@ If you cannot make the changes above, but still want to try out\nNext.js v13 wit require('../build/swc') as typeof import('../build/swc') const { eventCliSession } = require('../telemetry/events/version') as typeof import('../telemetry/events/version') - const { findPagesDir } = - require('../lib/find-pages-dir') as typeof import('../lib/find-pages-dir') const { setGlobal } = require('../trace') as typeof import('../trace') - const { Telemetry } = - require('../telemetry/storage') as typeof import('../telemetry/storage') + require('../telemetry/storage') as typeof import('../telemetry/storage') const findUp = require('next/dist/compiled/find-up') as typeof import('next/dist/compiled/find-up') diff --git a/packages/next/server/dev/next-dev-server.ts b/packages/next/server/dev/next-dev-server.ts index 4f126062f435109..6741856f069b5be 100644 --- a/packages/next/server/dev/next-dev-server.ts +++ b/packages/next/server/dev/next-dev-server.ts @@ -732,6 +732,12 @@ export default class DevServer extends Server { } const telemetry = new Telemetry({ distDir: this.distDir }) + + // This is required by the tracing subsystem. + setGlobal('appDir', this.appDir) + setGlobal('pagesDir', this.pagesDir) + setGlobal('telemetry', telemetry) + const isSrcDir = relative( this.dir, this.pagesDir || this.appDir || '' @@ -748,10 +754,6 @@ export default class DevServer extends Server { appDir: !!this.appDir, }) ) - // This is required by the tracing subsystem. - setGlobal('appDir', this.appDir) - setGlobal('pagesDir', this.pagesDir) - setGlobal('telemetry', telemetry) process.on('unhandledRejection', (reason) => { this.logErrorWithOriginalStack(reason, 'unhandledRejection').catch( diff --git a/test/integration/telemetry/test/index.test.js b/test/integration/telemetry/test/index.test.js index 0f3abcca358831a..dd33274444d5edc 100644 --- a/test/integration/telemetry/test/index.test.js +++ b/test/integration/telemetry/test/index.test.js @@ -467,8 +467,10 @@ describe('Telemetry CLI', () => { turbo: true, }) + await check(() => stderr, /NEXT_CLI_SESSION_STARTED/) + if (app) { - await killApp(app) + await app.kill('SIGTERM') } await check(() => stderr, /NEXT_CLI_SESSION_STOPPED/) @@ -503,7 +505,7 @@ describe('Telemetry CLI', () => { await check(() => stderr, /NEXT_CLI_SESSION_STARTED/) if (app) { - await killApp(app) + await app.kill('SIGTERM') } await check(() => stderr, /NEXT_CLI_SESSION_STOPPED/)