Skip to content

Commit fbe6452

Browse files
authored
fix: handle errors on plugins migration child process (#21442)
* fix: handle errors on plugins migration child process * Update error handler * Remove console.log
1 parent b4aa3d9 commit fbe6452

File tree

12 files changed

+67
-11
lines changed

12 files changed

+67
-11
lines changed

packages/data-context/src/actions/MigrationActions.ts

+15-4
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,16 @@ export async function processConfigViaLegacyPlugins (projectRoot: string, legacy
8888

8989
const configProcessArgs = ['--projectRoot', projectRoot, '--file', cwd]
9090
const CHILD_PROCESS_FILE_PATH = require.resolve('@packages/server/lib/plugins/child/require_async_child')
91-
const ipc = new LegacyPluginsIpc(fork(CHILD_PROCESS_FILE_PATH, configProcessArgs, childOptions))
91+
92+
const childProcess = fork(CHILD_PROCESS_FILE_PATH, configProcessArgs, childOptions)
93+
const ipc = new LegacyPluginsIpc(childProcess)
94+
95+
childProcess.on('error', (error) => {
96+
error = getError('LEGACY_CONFIG_ERROR_DURING_MIGRATION', cwd, error)
97+
98+
reject(error)
99+
ipc.killChildProcess()
100+
})
92101

93102
const legacyConfigWithDefaults = getConfigWithDefaults(legacyConfig)
94103

@@ -107,17 +116,19 @@ export async function processConfigViaLegacyPlugins (projectRoot: string, legacy
107116
const legacyConfigWithChanges = _.merge(legacyConfig, diff)
108117

109118
resolve(legacyConfigWithChanges)
110-
ipc.childProcess.kill()
119+
ipc.killChildProcess()
111120
})
112121

113122
ipc.on('loadLegacyPlugins:error', (error) => {
123+
error = getError('LEGACY_CONFIG_ERROR_DURING_MIGRATION', cwd, error)
124+
114125
reject(error)
115-
ipc.childProcess.kill()
126+
ipc.killChildProcess()
116127
})
117128

118129
ipc.on('childProcess:unhandledError', (error) => {
119130
reject(error)
120-
ipc.childProcess.kill()
131+
ipc.killChildProcess()
121132
})
122133
})
123134
}

packages/data-context/src/data/LegacyPluginsIpc.ts

+9
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,13 @@ export class LegacyPluginsIpc extends EventEmitter {
3232
on (evt: string, listener: (...args: any[]) => void) {
3333
return super.on(evt, listener)
3434
}
35+
36+
killChildProcess () {
37+
this.childProcess.kill()
38+
this.childProcess.stdout?.removeAllListeners()
39+
this.childProcess.stderr?.removeAllListeners()
40+
this.childProcess.removeAllListeners()
41+
42+
this.removeAllListeners()
43+
}
3544
}

packages/errors/__snapshot-html__/LEGACY_CONFIG_ERROR_DURING_MIGRATION.html

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/errors/src/errors.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -1191,7 +1191,7 @@ export const AllCypressErrors = {
11911191

11921192
LEGACY_CONFIG_ERROR_DURING_MIGRATION: (file: string, error: Error) => {
11931193
return errTemplate`
1194-
Your ${fmt.highlight(file)} at ${fmt.path(`${file}`)} threw an error. ${fmt.stackTrace(error)}
1194+
Your ${fmt.highlight(file)} file threw an error. ${fmt.stackTrace(error)}
11951195
11961196
Please ensure your pluginsFile is valid and relaunch the migration tool to migrate to ${fmt.cypressVersion('10.0.0')}.
11971197
`

packages/launchpad/cypress/e2e/migration.cy.ts

+16-1
Original file line numberDiff line numberDiff line change
@@ -1339,6 +1339,14 @@ describe('Migration', { viewportWidth: 1200, retries: { openMode: 2, runMode: 2
13391339
cy.get('button').contains('Cancel').click()
13401340
cy.get('h2').should('not.contain', 'Change the existing spec file extension')
13411341
})
1342+
1343+
it('shows error if plugins file throws an error', () => {
1344+
scaffoldAndVisitLaunchpad('migration-e2e-plugins-throw-error')
1345+
1346+
cy.contains('cypress/plugins/index.js file threw an error.')
1347+
cy.contains('Please ensure your pluginsFile is valid and relaunch the migration tool to migrate to Cypress version 10.0.0.')
1348+
cy.contains('throw new Error(\'New error from plugin\')')
1349+
})
13421350
})
13431351

13441352
describe('Migrate custom config files', () => {
@@ -1509,6 +1517,13 @@ describe('Migrate custom config files', () => {
15091517
scaffoldAndVisitLaunchpad('migration-custom-config-file-with-existing-v10-config-file', ['--config-file', 'customConfig.json'])
15101518

15111519
cy.contains('There is both a customConfig.config.js and a customConfig.json file at the location below:')
1512-
cy.contains('ypress no longer supports customConfig.json, please remove it from your project.')
1520+
cy.contains('Cypress no longer supports customConfig.json, please remove it from your project.')
1521+
})
1522+
1523+
it('shows error if plugins file do not exist', () => {
1524+
scaffoldAndVisitLaunchpad('migration', ['--config-file', 'erroredConfigFiles/incorrectPluginsFile.json'])
1525+
1526+
cy.contains('foo/bar file threw an error.')
1527+
cy.contains('Please ensure your pluginsFile is valid and relaunch the migration tool to migrate to Cypress version 10.0.0.')
15131528
})
15141529
})

packages/server/lib/plugins/child/run_require_async_child.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -287,9 +287,7 @@ function run (ipc, file, projectRoot) {
287287

288288
ipc.send('loadLegacyPlugins:reply', mergedLegacyConfig)
289289
} catch (e) {
290-
ipc.send('loadLegacyPlugins:error', util.serializeError(
291-
require('@packages/errors').getError('LEGACY_CONFIG_ERROR_DURING_MIGRATION', file, e),
292-
))
290+
ipc.send('loadLegacyPlugins:error', util.serializeError(e))
293291
}
294292
})
295293

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
## Migration E2E Plugins Throw Error
2+
3+
An e2e project where `cypress/plugins/index.js` modifies throws an error is handled correctly
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
/* eslint-disable */
2+
module.exports = (on, config) => {
3+
throw new Error('New error from plugin')
4+
5+
return config
6+
}

system-tests/projects/migration-e2e-plugins-throw-error/cypress/support/index.js

Whitespace-only changes.

system-tests/projects/migration-e2e-plugins-throw-error/tests/e2e/foo.spec.js

Whitespace-only changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"baseUrl": "http://localhost:3000",
3+
"retries": 2,
4+
"defaultCommandTimeout": 5000,
5+
"fixturesFolder": false,
6+
"componentFolder": "src",
7+
"testFiles": "**/*.spec.{tsx,js}",
8+
"pluginsFile": "foo/bar",
9+
"e2e": {
10+
"defaultCommandTimeout": 10000,
11+
"slowTestThreshold": 5000
12+
}
13+
}

0 commit comments

Comments
 (0)