From 570f91dde3a8bd54fd059e1cfe0f85bab8f1a7cb Mon Sep 17 00:00:00 2001 From: Tyler Biethman Date: Mon, 6 Dec 2021 15:31:25 -0600 Subject: [PATCH] fix: adding timeout option to writeFile command (#19015) --- cli/types/cypress.d.ts | 13 +-- .../integration/commands/files_spec.js | 100 +++++++++++++++++- packages/driver/src/cy/commands/files.ts | 59 +++++++---- packages/socket/lib/socket.ts | 16 +-- 4 files changed, 149 insertions(+), 39 deletions(-) diff --git a/cli/types/cypress.d.ts b/cli/types/cypress.d.ts index 66e54176f6..8a04619ad5 100644 --- a/cli/types/cypress.d.ts +++ b/cli/types/cypress.d.ts @@ -2209,12 +2209,9 @@ declare namespace Cypress { * @see https://on.cypress.io/writefile ``` cy.writeFile('path/to/message.txt', 'Hello World') - .then((text) => { - expect(text).to.equal('Hello World') // true - }) ``` */ - writeFile(filePath: string, contents: C, encoding: Encodings): Chainable + writeFile(filePath: string, contents: FileContents, encoding: Encodings): Chainable /** * Write to a file with the specified encoding and contents. * @@ -2223,12 +2220,10 @@ declare namespace Cypress { cy.writeFile('path/to/ascii.txt', 'Hello World', { flag: 'a+', encoding: 'ascii' - }).then((text) => { - expect(text).to.equal('Hello World') // true }) ``` */ - writeFile(filePath: string, contents: C, options?: Partial): Chainable + writeFile(filePath: string, contents: FileContents, options?: Partial): Chainable /** * Write to a file with the specified encoding and contents. * @@ -2238,12 +2233,10 @@ declare namespace Cypress { ``` cy.writeFile('path/to/ascii.txt', 'Hello World', 'utf8', { flag: 'a+', - }).then((text) => { - expect(text).to.equal('Hello World') // true }) ``` */ - writeFile(filePath: string, contents: C, encoding: Encodings, options?: Partial): Chainable + writeFile(filePath: string, contents: FileContents, encoding: Encodings, options?: Partial): Chainable /** * jQuery library bound to the AUT diff --git a/packages/driver/cypress/integration/commands/files_spec.js b/packages/driver/cypress/integration/commands/files_spec.js index e5ee3834b1..d140435790 100644 --- a/packages/driver/cypress/integration/commands/files_spec.js +++ b/packages/driver/cypress/integration/commands/files_spec.js @@ -145,6 +145,7 @@ describe('src/cy/commands/files', () => { if (attrs.name === 'readFile') { expect(log.get('state')).to.eq('pending') expect(log.get('message')).to.eq('foo.json') + expect(log.get('timeout')).to.eq(Cypress.config('defaultCommandTimeout')) } }) @@ -321,6 +322,54 @@ describe('src/cy/commands/files', () => { cy.readFile('foo.json').should('equal', 'contents') }) + + it('throws when the read timeout expires', function (done) { + Cypress.backend.callsFake(() => { + return new Cypress.Promise(() => { /* Broken promise for timeout */ }) + }) + + cy.on('fail', (err) => { + const { lastLog } = this + + assertLogLength(this.logs, 1) + expect(lastLog.get('error')).to.eq(err) + expect(lastLog.get('state')).to.eq('failed') + expect(err.message).to.eq(stripIndent`\ + \`cy.readFile("foo")\` timed out after waiting \`10ms\`. + `) + + expect(err.docsUrl).to.eq('https://on.cypress.io/readfile') + + done() + }) + + cy.readFile('foo', { timeout: 10 }) + }) + + it('uses defaultCommandTimeout config value if option not provided', { + defaultCommandTimeout: 42, + }, function (done) { + Cypress.backend.callsFake(() => { + return new Cypress.Promise(() => { /* Broken promise for timeout */ }) + }) + + cy.on('fail', (err) => { + const { lastLog } = this + + assertLogLength(this.logs, 1) + expect(lastLog.get('error')).to.eq(err) + expect(lastLog.get('state')).to.eq('failed') + expect(err.message).to.eq(stripIndent`\ + \`cy.readFile("foo")\` timed out after waiting \`42ms\`. + `) + + expect(err.docsUrl).to.eq('https://on.cypress.io/readfile') + + done() + }) + + cy.readFile('foo') + }) }) }) @@ -394,7 +443,7 @@ describe('src/cy/commands/files', () => { Cypress.backend.resolves(okResponse) cy.writeFile('foo.txt', 'contents').then((subject) => { - expect(subject).to.not.exist + expect(subject).to.eq(null) }) }) @@ -481,6 +530,7 @@ describe('src/cy/commands/files', () => { if (attrs.name === 'writeFile') { expect(log.get('state')).to.eq('pending') expect(log.get('message')).to.eq('foo.txt', 'contents') + expect(log.get('timeout')).to.eq(Cypress.config('defaultCommandTimeout')) } }) @@ -601,6 +651,54 @@ describe('src/cy/commands/files', () => { cy.writeFile('foo.txt', 'contents') }) + + it('throws when the write timeout expires', function (done) { + Cypress.backend.callsFake(() => { + return new Cypress.Promise(() => {}) + }) + + cy.on('fail', (err) => { + const { lastLog } = this + + assertLogLength(this.logs, 1) + expect(lastLog.get('error')).to.eq(err) + expect(lastLog.get('state')).to.eq('failed') + expect(err.message).to.eq(stripIndent` + \`cy.writeFile("foo.txt")\` timed out after waiting \`10ms\`. + `) + + expect(err.docsUrl).to.eq('https://on.cypress.io/writefile') + + done() + }) + + cy.writeFile('foo.txt', 'contents', { timeout: 10 }) + }) + + it('uses defaultCommandTimeout config value if option not provided', { + defaultCommandTimeout: 42, + }, function (done) { + Cypress.backend.callsFake(() => { + return new Cypress.Promise(() => { /* Broken promise for timeout */ }) + }) + + cy.on('fail', (err) => { + const { lastLog } = this + + assertLogLength(this.logs, 1) + expect(lastLog.get('error')).to.eq(err) + expect(lastLog.get('state')).to.eq('failed') + expect(err.message).to.eq(stripIndent` + \`cy.writeFile("foo.txt")\` timed out after waiting \`42ms\`. + `) + + expect(err.docsUrl).to.eq('https://on.cypress.io/writefile') + + done() + }) + + cy.writeFile('foo.txt', 'contents') + }) }) }) }) diff --git a/packages/driver/src/cy/commands/files.ts b/packages/driver/src/cy/commands/files.ts index 276ccfa766..7bf4c2db36 100644 --- a/packages/driver/src/cy/commands/files.ts +++ b/packages/driver/src/cy/commands/files.ts @@ -1,6 +1,5 @@ // @ts-nocheck import _ from 'lodash' -import Promise from 'bluebird' import $errUtils from '../../cypress/error_utils' @@ -22,6 +21,7 @@ export default (Commands, Cypress, cy) => { // to restore the default node behavior. encoding: encoding === undefined ? 'utf8' : encoding, log: true, + timeout: Cypress.config('defaultCommandTimeout'), }) const consoleProps = {} @@ -43,21 +43,33 @@ export default (Commands, Cypress, cy) => { }) } + // We clear the default timeout so we can handle + // the timeout ourselves + cy.clearTimeout() + const verifyAssertions = () => { - return Cypress.backend('read:file', file, _.pick(options, 'encoding')) + return Cypress.backend('read:file', file, _.pick(options, 'encoding')).timeout(options.timeout) .catch((err) => { - if (err.code === 'ENOENT') { - return { - contents: null, - filePath: err.filePath, - } + if (err.name === 'TimeoutError') { + return $errUtils.throwErrByPath('files.timed_out', { + onFail: options._log, + args: { cmd: 'readFile', file, timeout: options.timeout }, + }) } - return $errUtils.throwErrByPath('files.unexpected_error', { - onFail: options._log, - args: { cmd: 'readFile', action: 'read', file, filePath: err.filePath, error: err.message }, - }) - }).then(({ contents, filePath }) => { + // Non-ENOENT errors are not retried + if (err.code !== 'ENOENT') { + return $errUtils.throwErrByPath('files.unexpected_error', { + onFail: options._log, + args: { cmd: 'readFile', action: 'read', file, filePath: err.filePath, error: err.message }, + }) + } + + return { + contents: null, + filePath: err.filePath, + } + }).then(({ filePath, contents }) => { // https://github.com/cypress-io/cypress/issues/1558 // We invoke Buffer.from() in order to transform this from an ArrayBuffer - // which socket.io uses to transfer the file over the websocket - into a @@ -110,6 +122,7 @@ export default (Commands, Cypress, cy) => { encoding: encoding === undefined ? 'utf8' : encoding, flag: userOptions.flag ? userOptions.flag : 'w', log: true, + timeout: Cypress.config('defaultCommandTimeout'), }) const consoleProps = {} @@ -117,7 +130,7 @@ export default (Commands, Cypress, cy) => { if (options.log) { options._log = Cypress.log({ message: fileName, - timeout: 0, + timeout: options.timeout, consoleProps () { return consoleProps }, @@ -142,19 +155,25 @@ export default (Commands, Cypress, cy) => { contents = JSON.stringify(contents, null, 2) } - return Cypress.backend('write:file', fileName, contents, _.pick(options, ['encoding', 'flag'])) - .then(({ contents, filePath }) => { + // We clear the default timeout so we can handle + // the timeout ourselves + cy.clearTimeout() + + return Cypress.backend('write:file', fileName, contents, _.pick(options, 'encoding', 'flag')).timeout(options.timeout) + .then(({ filePath, contents }) => { consoleProps['File Path'] = filePath consoleProps['Contents'] = contents return null - }).catch(Promise.TimeoutError, () => { - return $errUtils.throwErrByPath('files.timed_out', { - onFail: options._log, - args: { cmd: 'writeFile', file: fileName, timeout: options.timeout }, - }) }) .catch((err) => { + if (err.name === 'TimeoutError') { + return $errUtils.throwErrByPath('files.timed_out', { + onFail: options._log, + args: { cmd: 'writeFile', file: fileName, timeout: options.timeout }, + }) + } + return $errUtils.throwErrByPath('files.unexpected_error', { onFail: options._log, args: { cmd: 'writeFile', action: 'write', file: fileName, filePath: err.filePath, error: err.message }, diff --git a/packages/socket/lib/socket.ts b/packages/socket/lib/socket.ts index e93099df99..e0c6a08a9d 100644 --- a/packages/socket/lib/socket.ts +++ b/packages/socket/lib/socket.ts @@ -1,10 +1,9 @@ import fs from 'fs' +import buffer from 'buffer' import type http from 'http' import server, { Server as SocketIOBaseServer, ServerOptions } from 'socket.io' import { client } from './browser' -const HUNDRED_MEGABYTES = 1e8 // 100000000 - const { version } = require('socket.io-client/package.json') const clientSource = require.resolve('socket.io-client/dist/socket.io.js') @@ -15,13 +14,14 @@ type PatchedServerOptions = ServerOptions & { cookie: { name: string | boolean } class SocketIOServer extends SocketIOBaseServer { constructor (srv: http.Server, opts?: Partial) { - // in socket.io v3, they reduced down the max buffer size - // from 100mb to 1mb, so we reset it back to the previous value - // - // previous commit for reference: - // https://github.com/socketio/engine.io/blame/61b949259ed966ef6fc8bfd61f14d1a2ef06d319/lib/server.js#L29 opts = opts ?? {} - opts.maxHttpBufferSize = opts.maxHttpBufferSize ?? HUNDRED_MEGABYTES + + // the maxHttpBufferSize is used to limit the message size sent over + // the socket. Small values can be used to mitigate exposure to + // denial of service attacks; the default as of v3.0 is 1MB. + // because our server is local, we do not need to arbitrarily limit + // the message size and can use the theoretical maximum value. + opts.maxHttpBufferSize = opts.maxHttpBufferSize ?? buffer.constants.MAX_LENGTH super(srv, opts) }