Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: suppress EventEmitter warnings + other Node warnings in prod #15723

Merged
merged 5 commits into from
Apr 1, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ process.noDeprecation = process.env.CYPRESS_INTERNAL_ENV === 'production'
// always show stack traces for Electron deprecation warnings
process.traceDeprecation = true

require('./lib/util/suppress_unauthorized_warning').suppress()
require('./lib/util/suppress_warnings').suppress()

function launchOrFork () {
const nodeOptions = require('./lib/util/node_options')
Expand Down
17 changes: 12 additions & 5 deletions packages/server/lib/browsers/electron.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ module.exports = {
return this._enableDebugger(win.webContents)
})
.then(() => {
return this._handleDownloads(win.webContents, options.downloadsFolder, automation)
return this._handleDownloads(win, options.downloadsFolder, automation)
})
.return(win)
},
Expand Down Expand Up @@ -291,8 +291,8 @@ module.exports = {
return webContents.debugger.sendCommand('Console.enable')
},

_handleDownloads (webContents, dir, automation) {
webContents.session.on('will-download', (event, downloadItem) => {
_handleDownloads (win, dir, automation) {
const onWillDownload = (event, downloadItem) => {
const savePath = path.join(dir, downloadItem.getFilename())

automation.push('create:download', {
Expand All @@ -307,9 +307,16 @@ module.exports = {
id: downloadItem.getETag(),
})
})
})
}

const { session } = win.webContents

session.on('will-download', onWillDownload)

// avoid adding redundant `will-download` handlers if session is reused for next spec
win.on('closed', () => session.removeListener('will-download', onWillDownload))
Comment on lines +316 to +317
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apparently the session is reused between webContents, because this eventually emits the same max listeners warning after 12 or so specfiles have launched Electron


return webContents.debugger.sendCommand('Page.setDownloadBehavior', {
return win.webContents.debugger.sendCommand('Page.setDownloadBehavior', {
behavior: 'allow',
downloadPath: dir,
})
Expand Down
2 changes: 1 addition & 1 deletion packages/server/lib/plugins/child/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require('graceful-fs').gracefulify(require('fs'))

require('../../util/suppress_unauthorized_warning').suppress()
require('../../util/suppress_warnings').suppress()

const ipc = require('../util').wrapIpc(process)
const { file: pluginsFile, projectRoot } = require('minimist')(process.argv.slice(2))
Expand Down
4 changes: 4 additions & 0 deletions packages/server/lib/plugins/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ module.exports = {
return emitter.emit(message.event, ...message.args)
})

// prevent max listeners warning on ipc
// @see https://github.com/cypress-io/cypress/issues/1305#issuecomment-780895569
emitter.setMaxListeners(Infinity)

return {
send (event, ...args) {
if (aProcess.killed) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const _ = require('lodash')
const Debug = require('debug')

const originalEmitWarning = process.emitWarning
const debug = Debug('cypress:server:lib:util:suppress_warnings')

let suppressed = false

Expand All @@ -10,15 +11,16 @@ let suppressed = false
* https://github.com/cypress-io/cypress/issues/5248
*/
const suppress = () => {
if (suppressed) {
if (suppressed || process.env.CYPRESS_INTERNAL_ENV === 'production') {
// in development, still log warnings since they are helpful
return
}

suppressed = true

process.emitWarning = (warning, type, code, ...args) => {
if (_.isString(warning) && _.includes(warning, 'NODE_TLS_REJECT_UNAUTHORIZED')) {
// https://github.com/nodejs/node/blob/82f89ec8c1554964f5029fab1cf0f4fad1fa55a8/lib/_tls_wrap.js#L1378-L1384
// https://github.com/nodejs/node/blob/85e6089c4db4da23dd88358fe0a12edefcd411f2/lib/internal/options.js#L17

return
}
Expand All @@ -31,7 +33,7 @@ const suppress = () => {
return
}

return originalEmitWarning.call(process, warning, type, code, ...args)
debug('suppressed emitWarning from node: %o', { process, warning, type, code, args })
}
}

Expand Down
35 changes: 35 additions & 0 deletions packages/server/test/e2e/0_max_listeners_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import _ from 'lodash'
import path from 'path'
import fs from 'fs-extra'

import e2e from '../support/helpers/e2e'
import Fixtures from '../support/helpers/fixtures'

const projectPath = Fixtures.projectPath('max-listeners')

describe('max listeners warning spec', () => {
beforeEach(() => {
e2e.setup()
})

// @see https://github.com/cypress-io/cypress/issues/1305
e2e.it('does not log MaxEventListeners error', {
browser: 'electron',
project: projectPath,
spec: '*',
onRun: async (exec) => {
const integrationPath = path.join(projectPath, 'cypress/integration')

// create a bunch of dummy test files to reproduce #1305
await fs.mkdirp(integrationPath)
await Promise.all(
_.times(15, (i) => fs.writeFile(path.join(integrationPath, `${i}.spec.js`), `it('test', () => {})`)),
)

const { stderr } = await exec()

expect(stderr).to.not.include('setMaxListeners')
expect(stderr).to.not.include('preprocessor:close')
},
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
1 change: 0 additions & 1 deletion packages/server/test/support/helpers/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,6 @@ const e2e = {

if (process.env.NO_EXIT) {
Fixtures.scaffoldWatch()
process.env.CYPRESS_INTERNAL_E2E_TESTS
}

sinon.stub(process, 'exit')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import proxyquire from 'proxyquire'
const ERROR_MESSAGE = 'Setting the NODE_TLS_REJECT_UNAUTHORIZED'

const TLS_CONNECT = `require('tls').connect().on('error', ()=>{});`
const SUPPRESS_WARNING = `require('${__dirname}/../../lib/util/suppress_unauthorized_warning').suppress();`
const SUPPRESS_WARNING = `require('${__dirname}/../../lib/util/suppress_warnings').suppress();`

describe('lib/util/suppress_unauthorized_warning', function () {
describe('lib/util/suppress_warnings', function () {
it('tls.connect emits warning if NODE_TLS_REJECT_UNAUTHORIZED=0 and not suppressed', function () {
return execa.shell(`node -e "${TLS_CONNECT}"`, {
env: {
Expand Down Expand Up @@ -36,7 +36,7 @@ describe('lib/util/suppress_unauthorized_warning', function () {
const emitWarning = sinon.spy(process, 'emitWarning')

// force typescript to always be non-requireable
const { suppress } = proxyquire('../../lib/util/suppress_unauthorized_warning', {})
const { suppress } = proxyquire('../../lib/util/suppress_warnings', {})

suppress()

Expand Down