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

Cacie/fix/proxy performance #31283

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
63 changes: 47 additions & 16 deletions packages/server/lib/cloud/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,19 @@ const request = require('@cypress/request-promise')
const humanInterval = require('human-interval')

const RequestErrors = require('@cypress/request-promise/errors')
const { agent } = require('@packages/network')

const pkg = require('@packages/root')

const machineId = require('../machine_id')
const errors = require('../../errors')
const { apiUrl, apiRoutes, makeRoutes } = require('../routes')

import Bluebird from 'bluebird'

import type { AfterSpecDurations } from '@packages/types'
import { agent } from '@packages/network'
import type { CombinedAgent } from '@packages/network/lib/agent'

import { apiUrl, apiRoutes, makeRoutes } from '../routes'
import { getText } from '../../util/status_code'
import * as enc from '../encryption'
import getEnvInformationForProjectRoot from '../environment'
Expand All @@ -22,7 +27,7 @@ import type { OptionsWithUrl } from 'request-promise'
import { fs } from '../../util/fs'
import ProtocolManager from '../protocol'
import type { ProjectBase } from '../../project-base'
import type { AfterSpecDurations } from '@packages/types'

import { PUBLIC_KEY_VERSION } from '../constants'

import { createInstance } from './create_instance'
Expand Down Expand Up @@ -227,6 +232,15 @@ const isRetriableError = (err) => {
(err.statusCode == null)
}

function noproxyPreflightTimeout (): number {
try {
return !_.isUndefined(process.env.CYPRESS_INTERNAL_INITIAL_PREFLIGHT_TIMEOUT) ?
Copy link
Member

Choose a reason for hiding this comment

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

Is this not intended for users to set? CYPRESS_INTERNAL is intended to never be used by users.

Number(process.env.CYPRESS_INTERNAL_PREFLIGHT_TIMEOUT) : SIXTY_SECONDS
} catch (e: unknown) {
return SIXTY_SECONDS
}
}

export type CreateRunOptions = {
projectRoot: string
ci: {
Expand Down Expand Up @@ -299,8 +313,21 @@ type UpdateInstanceArtifactsOptions = {
instanceId: string
timeout?: number
}
interface DefaultPreflightResult {
encrypt: true
}

let preflightResult = {
interface PreflightWarning {
message: string
}

interface CachedPreflightResult {
encrypt: boolean
apiUrl: string
warnings?: PreflightWarning[]
}

let preflightResult: DefaultPreflightResult | CachedPreflightResult = {
encrypt: true,
}

Expand Down Expand Up @@ -572,28 +599,27 @@ export default {

sendPreflight (preflightInfo) {
return retryWithBackoff(async (attemptIndex) => {
const { timeout, projectRoot } = preflightInfo

preflightInfo = _.omit(preflightInfo, 'timeout', 'projectRoot')
const { projectRoot, timeout, ...preflightRequestBody } = preflightInfo

const preflightBaseProxy = apiUrl.replace('api', 'api-proxy')

const envInformation = await getEnvInformationForProjectRoot(projectRoot, process.pid.toString())
const makeReq = ({ baseUrl, agent }) => {

const makeReq = (baseUrl: string, agent: CombinedAgent | null, timeout: number) => {
return rp.post({
url: `${baseUrl}preflight`,
body: {
apiUrl,
envUrl: envInformation.envUrl,
dependencies: envInformation.dependencies,
errors: envInformation.errors,
...preflightInfo,
...preflightRequestBody,
},
headers: {
'x-route-version': '1',
'x-cypress-request-attempt': attemptIndex,
},
timeout: timeout ?? SIXTY_SECONDS,
timeout,
json: true,
encrypt: 'always',
agent,
Expand All @@ -605,14 +631,19 @@ export default {
}

const postReqs = async () => {
return makeReq({ baseUrl: preflightBaseProxy, agent: null })
.catch((err) => {
if (err.statusCode === 412) {
throw err
const initialPreflightTimeout = noproxyPreflightTimeout()

if (initialPreflightTimeout >= 0) {
try {
return await makeReq(preflightBaseProxy, null, noproxyPreflightTimeout())
} catch (err) {
if (err.statusCode === 412) {
throw err
}
}
}

return makeReq({ baseUrl: apiUrl, agent })
})
return makeReq(apiUrl, agent, timeout)
}

const result = await postReqs()
Expand Down
13 changes: 5 additions & 8 deletions packages/server/lib/cloud/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import _ from 'lodash'
import UrlParse from 'url-parse'

const app_config = require('../../config/app.json')
const apiUrl = app_config[process.env.CYPRESS_CONFIG_ENV || process.env.CYPRESS_INTERNAL_ENV || 'development'].api_url

export const apiUrl = app_config[process.env.CYPRESS_CONFIG_ENV || process.env.CYPRESS_INTERNAL_ENV || 'development'].api_url

const CLOUD_ENDPOINTS = {
api: '',
Expand Down Expand Up @@ -39,7 +40,7 @@ const parseArgs = function (url, args: any[] = []) {
return url
}

const makeRoutes = (baseUrl: string, routes: typeof CLOUD_ENDPOINTS) => {
const _makeRoutes = (baseUrl: string, routes: typeof CLOUD_ENDPOINTS) => {
return _.reduce(routes, (memo, value, key) => {
memo[key] = function (...args: any[]) {
let url = new UrlParse(baseUrl, true)
Expand All @@ -59,10 +60,6 @@ const makeRoutes = (baseUrl: string, routes: typeof CLOUD_ENDPOINTS) => {
}, {} as Record<keyof typeof CLOUD_ENDPOINTS, (...args: any[]) => string>)
}

const apiRoutes = makeRoutes(apiUrl, CLOUD_ENDPOINTS)
export const apiRoutes = _makeRoutes(apiUrl, CLOUD_ENDPOINTS)

module.exports = {
apiUrl,
apiRoutes,
makeRoutes: (baseUrl) => makeRoutes(baseUrl, CLOUD_ENDPOINTS),
}
export const makeRoutes = (baseUrl) => _makeRoutes(baseUrl, CLOUD_ENDPOINTS)
67 changes: 66 additions & 1 deletion packages/server/test/unit/cloud/api/api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,8 @@ describe('lib/cloud/api', () => {
require('../../../../lib/cloud/encryption')
}, module)
}

prodApi.resetPreflightResult()
})

it('POST /preflight to proxy. returns encryption', () => {
Expand Down Expand Up @@ -323,7 +325,7 @@ describe('lib/cloud/api', () => {
})
})

it('sets timeout to 60 seconds', () => {
it('sets timeout to 60 seconds when no CYPRESS_INTERNAL_INITIAL_PREFLIGHT_TIMEOUT env is set', () => {
sinon.stub(api.rp, 'post').resolves({})

return api.sendPreflight({})
Expand All @@ -332,6 +334,69 @@ describe('lib/cloud/api', () => {
})
})

describe('when CYPRESS_INTERNAL_INITIAL_PREFLIGHT_TIMEOUT env is set to a negative number', () => {
const configuredTimeout = -1
let prevEnv

beforeEach(() => {
prevEnv = process.env.CYPRESS_INTERNAL_INITIAL_PREFLIGHT_TIMEOUT
process.env.CYPRESS_INTERNAL_INITIAL_PREFLIGHT_TIMEOUT = configuredTimeout
})

afterEach(() => {
process.env.CYPRESS_INTERNAL_INITIAL_PREFLIGHT_TIMEOUT = prevEnv
})

it('skips the no-agent preflight request', () => {
preflightNock(API_PROD_PROXY_BASEURL)
.replyWithError('should not be called')

preflightNock(API_PROD_BASEURL)
.reply(200, decryptReqBodyAndRespond({
reqBody: {
envUrl: 'https://some.server.com',
dependencies: {},
errors: [],
apiUrl: 'https://api.cypress.io/',
projectId: 'abc123',
},
resBody: {
encrypt: true,
apiUrl: `${API_PROD_BASEURL}/`,
},
}))

return prodApi.sendPreflight({ projectId: 'abc123' })
.then((ret) => {
expect(ret).to.deep.eq({ encrypt: true, apiUrl: `${API_PROD_BASEURL}/` })
})
})
})

describe('when CYPRESS_INTERNAL_INITIAL_PREFLIGHT_TIMEOUT env is set to a positive number', () => {
const configuredTimeout = 10000
let prevEnv

beforeEach(() => {
prevEnv = process.env.CYPRESS_INTERNAL_INITIAL_PREFLIGHT_TIMEOUT
process.env.CYPRESS_INTERNAL_INITIAL_PREFLIGHT_TIMEOUT = configuredTimeout
})

afterEach(() => {
process.env.CYPRESS_INTERNAL_INITIAL_PREFLIGHT_TIMEOUT = prevEnv
api.rp.post.restore()
})

it('makes the initial request with the number set in the env', () => {
sinon.stub(api.rp, 'post').resolves({})

return api.sendPreflight({})
.then(() => {
expect(api.rp.post).to.be.calledWithMatch({ timeout: configuredTimeout })
})
})
})

describe('errors', () => {
it('[F1] POST /preflight TimeoutError', () => {
preflightNock(API_BASEURL)
Expand Down
Loading