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

PAC file support via PERCY_PAC_FILE_URL env var #1784

Merged
merged 7 commits into from
Nov 19, 2024
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
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@
"@babel/eslint-parser": "^7.14.7",
"@babel/preset-env": "^7.14.7",
"@babel/register": "^7.17.7",
"babel-plugin-transform-import-meta": "^2.2.1",
"@rollup/plugin-alias": "^5.1.1",
"@rollup/plugin-babel": "^6.0.0",
"@rollup/plugin-commonjs": "^21.0.0",
"@rollup/plugin-node-resolve": "^15.0.0",
"babel-plugin-istanbul": "^6.0.0",
"babel-plugin-module-resolver": "^4.1.0",
"babel-plugin-transform-import-meta": "^2.2.1",
"cross-env": "^7.0.2",
"eslint": "^7.30.0",
"eslint-config-standard": "^16.0.2",
Expand All @@ -59,5 +59,8 @@
"nyc": "^15.1.0",
"rollup": "^2.53.2",
"tsd": "^0.31.2"
},
"dependencies": {
"pac-proxy-agent": "^7.0.2"
ninadbstack marked this conversation as resolved.
Show resolved Hide resolved
}
}
48 changes: 43 additions & 5 deletions packages/client/src/proxy.js
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing specs

Copy link
Author

@khushhalm khushhalm Nov 19, 2024

Choose a reason for hiding this comment

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

Can I do this later? Can create a Jira so that this is not missed.
Need to share the custom build to GM by today to test PAC support.

Or if it blocker can I take help from someone in Percy team for this? Since need to test couple of other things.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, we dont merge anything without specs on CLI - I can release alpha version from branch without specs if you want - so that its not merged to master

Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,28 @@ import http from 'http';
import https from 'https';
import logger from '@percy/logger';
import { stripQuotesAndSpaces } from '@percy/env/utils';
import { PacProxyAgent } from 'pac-proxy-agent';
ninadbstack marked this conversation as resolved.
Show resolved Hide resolved

const CRLF = '\r\n';
const STATUS_REG = /^HTTP\/1.[01] (\d*)/;

// function to create PAC proxy agent
function createPacAgent(pacUrl, options = {}) {
try {
const agent = new PacProxyAgent(pacUrl, {
keepAlive: true,
...options,
rejectUnauthorized: false
khushhalm marked this conversation as resolved.
Show resolved Hide resolved
});

logger('client:proxy').info(`Successfully loaded PAC file from: ${pacUrl}`);
return agent;
} catch (error) {
logger('client:proxy').error(`Failed to load PAC file, error message: ${error.message}, stack: ${error.stack}`);
throw new Error(`Failed to initialize PAC proxy: ${error.message}`);
}
}

// Returns true if the URL hostname matches any patterns
export function hostnameMatches(patterns, url) {
let subject = new URL(url);
Expand Down Expand Up @@ -219,11 +237,31 @@ export function proxyAgentFor(url, options) {
let { protocol, hostname } = new URL(url);
let cachekey = `${protocol}//${hostname}`;

if (!cache.has(cachekey)) {
cache.set(cachekey, protocol === 'https:'
? new ProxyHttpsAgent(options)
: new ProxyHttpAgent(options));
// If we already have a cached agent, return it
if (cache.has(cachekey)) {
return cache.get(cachekey);
}

return cache.get(cachekey);
try {
let agent;
const pacUrl = process.env.PERCY_PAC_FILE_PATH;
khushhalm marked this conversation as resolved.
Show resolved Hide resolved

// If PAC URL is provided, use PAC proxy
if (pacUrl) {
logger('client:proxy').info(`Using PAC file from: ${pacUrl}`);
agent = createPacAgent(stripQuotesAndSpaces(pacUrl), options);
khushhalm marked this conversation as resolved.
Show resolved Hide resolved
} else {
// Fall back to other proxy configuration
agent = protocol === 'https:'
? new ProxyHttpsAgent(options)
: new ProxyHttpAgent(options);
}

// Cache the created agent
cache.set(cachekey, agent);
return agent;
} catch (error) {
logger('client:proxy').error(`Failed to create proxy agent: ${error.message}`);
throw error;
}
}
7 changes: 3 additions & 4 deletions packages/core/src/percy.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ import { getProxy } from '@percy/client/utils';
import Browser from './browser.js';
import Pako from 'pako';
import {
base64encode
,
base64encode,
generatePromise,
yieldAll,
yieldTo
, redactSecrets,
yieldTo,
redactSecrets,
detectSystemProxyAndLog
} from './utils.js';

Expand Down
Loading