Skip to content

Commit

Permalink
Merge branch 'master' into SNOW-1814745-Fix-aborting-requests-in-Http…
Browse files Browse the repository at this point in the history
…Client
  • Loading branch information
sfc-gh-fpawlowski authored Dec 16, 2024
2 parents a196604 + 0aaa740 commit 7a594c5
Show file tree
Hide file tree
Showing 18 changed files with 499 additions and 102 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ jobs:
fail-fast: false
matrix:
cloud: [ 'AWS', 'AZURE', 'GCP' ]
nodeVersion: [ '14.x', '16.x', '18.x', '20.x', '22.x']
nodeVersion: ['18.x', '20.x', '22.x']
steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
Expand Down Expand Up @@ -87,7 +87,7 @@ jobs:
fail-fast: false
matrix:
cloud: [ 'AWS', 'AZURE', 'GCP' ]
nodeVersion: [ '14.x', '16.x', '18.x', '20.x', '22.x']
nodeVersion: ['18.x', '20.x', '22.x']
steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/jira_issue.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
summary: '${{ github.event.issue.title }}'
description: |
${{ github.event.issue.body }} \\ \\ _Created from GitHub Action_ for ${{ github.event.issue.html_url }}
fields: '{ "customfield_11401": {"id": "14723"}, "assignee": {"id": "712020:3c0352b5-63f7-4e26-9afe-38f6f9f0f4c5"}, "components":[{"id":"19290"}] }'
fields: '{ "customfield_11401": {"id": "14723"}, "assignee": {"id": "712020:e1f41916-da57-4fe8-b317-116d5229aa51"}, "components":[{"id":"19290"}], "labels": ["oss"], "priority": {"id": "10001"} }'

- name: Update GitHub Issue
uses: ./jira/gajira-issue-update
Expand Down
2 changes: 1 addition & 1 deletion Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ timestamps {
'Test': {
stage('Test') {
def params = [
string(name: 'svn_revision', value: 'main'),
string(name: 'svn_revision', value: 'bptp-built'),
string(name: 'branch', value: 'main'),
string(name: 'client_git_commit', value: scmInfo.GIT_COMMIT),
string(name: 'client_git_branch', value: scmInfo.GIT_BRANCH),
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ at <a href="https://docs.snowflake.net/manuals/user-guide/nodejs-driver.html">No
Note
----------------------------------------------------------------------

This driver currently does not support GCP regional endpoints. Please ensure that any workloads using through this driver do not require support for regional endpoints on GCP. If you have questions about this, please contact Snowflake Support.

This driver starts supporting the GCS regional endpoint starting from version 2.0.0. Please ensure that any workloads using through this driver
below the version 2.0.0 do not require support for regional endpoints on GCP. If you have questions about this, please contact Snowflake Support.

Test
======================================================================
Expand Down
7 changes: 0 additions & 7 deletions ci/container/test_component.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,6 @@ npm install
PACKAGE_NAME=$(cd $WORKSPACE && ls snowflake-sdk*.tgz)
npm install $WORKSPACE/${PACKAGE_NAME}

# Since @azure lib has lost compatibility with node14
#some dependencies have to be replaced by an older version
nodeVersion=$(node -v)
if [[ "$nodeVersion" == 'v14.'* ]]; then
npm install @azure/[email protected]
fi

echo "[INFO] Setting test parameters"
if [[ "$LOCAL_USER_NAME" == "jenkins" ]]; then
echo "[INFO] Use the default test parameters.json"
Expand Down
8 changes: 0 additions & 8 deletions ci/test_windows.bat
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,6 @@ echo [INFO] Installing Snowflake NodeJS Driver
copy %GITHUB_WORKSPACE%\artifacts\* .
for %%f in (snowflake-sdk*.tgz) do cmd /c npm install %%f

REM Since @azure lib has lost compatibility with node14
REM some dependencies have to be replaced by an older version
FOR /F "tokens=* USEBACKQ" %%F IN (`node -v`) DO (
SET nodeVersion=%%F
)
ECHO %nodeVersion%
if not x%nodeVersion:v14.=%==x%str1% cmd /c npm install @azure/core-lro@2.6.0

if %ERRORLEVEL% NEQ 0 (
echo [ERROR] failed to install the Snowflake NodeJS Driver
exit /b 1
Expand Down
10 changes: 8 additions & 2 deletions lib/constants/sf_params.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,8 @@
exports.paramsNames = {};
exports.paramsNames.SF_REQUEST_GUID = 'request_guid';
exports.paramsNames = Object.freeze({
SF_REQUEST_GUID: 'request_guid',
SF_REQUEST_ID: 'requestId',
SF_TOKEN: 'token',
SF_WAREHOUSE_NAME: 'warehouse',
SF_DB_NAME: 'databaseName',
SF_SCHEMA_NAME: 'schemaName',
});
2 changes: 1 addition & 1 deletion lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ exports.createOperationFailedError = function (
return createError(errorTypes.OperationFailedError,
{
code: errorCode,
data: { ...data, queryId: null },
data: data,
message: message,
sqlState: sqlState
});
Expand Down
9 changes: 5 additions & 4 deletions lib/http/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const HttpsProxyAgent = require('../agent/https_proxy_agent');
const HttpAgent = require('http').Agent;
const GlobalConfig = require('../../lib/global_config');
const Logger = require('../logger');
const RequestUtil = require('../http/request_util');

/**
* Returns the delay time calculated by exponential backoff with
Expand Down Expand Up @@ -107,19 +108,19 @@ function isBypassProxy(proxy, destination, agentId) {
* @inheritDoc
*/
NodeHttpClient.prototype.getAgent = function (parsedUrl, proxy, mock) {
Logger.getInstance().trace('Agent[url: %s] - getting an agent instance.', parsedUrl.href);
Logger.getInstance().trace('Agent[url: %s] - getting an agent instance.', RequestUtil.describeURL(parsedUrl.href));
if (!proxy && GlobalConfig.isEnvProxyActive()) {
const isHttps = parsedUrl.protocol === 'https:';
proxy = ProxyUtil.getProxyFromEnv(isHttps);
if (proxy) {
Logger.getInstance().debug('Agent[url: %s] - proxy info loaded from the environment variable. Proxy host: %s', parsedUrl.href, proxy.host);
Logger.getInstance().debug('Agent[url: %s] - proxy info loaded from the environment variable. Proxy host: %s', RequestUtil.describeURL(parsedUrl.href), proxy.host);
}
}
return getProxyAgent(proxy, parsedUrl, parsedUrl.href, mock);
};

function getProxyAgent(proxyOptions, parsedUrl, destination, mock) {
Logger.getInstance().trace('Agent[url: %s] - getting a proxy agent instance.', parsedUrl.href);
Logger.getInstance().trace('Agent[url: %s] - getting a proxy agent instance.', RequestUtil.describeURL(parsedUrl.href));
const agentOptions = {
protocol: parsedUrl.protocol,
hostname: parsedUrl.hostname,
Expand All @@ -129,7 +130,7 @@ function getProxyAgent(proxyOptions, parsedUrl, destination, mock) {
if (mock) {
const mockAgent = mock.agentClass(agentOptions);
if (mockAgent.protocol === parsedUrl.protocol) {
Logger.getInstance().debug('Agent[url: %s] - the mock agent will be used.', parsedUrl.href);
Logger.getInstance().debug('Agent[url: %s] - the mock agent will be used.', RequestUtil.describeURL(parsedUrl.href));
return mockAgent;
}
}
Expand Down
215 changes: 190 additions & 25 deletions lib/http/request_util.js
Original file line number Diff line number Diff line change
@@ -1,37 +1,202 @@
const LoggingUtil = require('../logger/logging_util');
const sfParams = require('../constants/sf_params');

// Initial whitelist for attributes - they will be described with values
const DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES = [
'baseUrl',
'path',
'method',
sfParams.paramsNames.SF_REQUEST_ID,
sfParams.paramsNames.SF_REQUEST_GUID,
sfParams.paramsNames.SF_WAREHOUSE_NAME,
sfParams.paramsNames.SF_DB_NAME,
sfParams.paramsNames.SF_SCHEMA_NAME,
];

// Initial blacklist for attributes - described as present/not present only
const DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES = [
sfParams.paramsNames.SF_TOKEN
];

// Helper function to resolve attributes arrays given defaults and overrides.
function resolveAttributeList(defaultAttrs, overrideAttrs) {
return overrideAttrs || defaultAttrs;
}

/**
* Should work with not yet parsed options as well (before calling prepareRequestOptions method).
* Describes a request based on its options.
* Should work with not-yet-parsed options as well (before calling prepareRequestOptions method).
*
* @param requestOptions - object representing the request data with top-level keys
* @returns {string}
* @param {Object} requestOptions - Object representing the request data with top-level keys.
* @param {Object} [options] - Options for describing attributes.
* @param {Array<string>} [options.overrideAttributesDescribedWithValues]
* @param {Array<string>} [options.overrideAttributesDescribedWithoutValues]
* @returns {string} A string representation of the request data.
*/
exports.describeRequestFromOptions = function (requestOptions) {
return exports.describeRequestData({ method: requestOptions.method, url: requestOptions.url, guid: requestOptions.params?.[sfParams.paramsNames.SF_REQUEST_GUID] });
};
function describeRequestFromOptions(
requestOptions,
{
overrideAttributesDescribedWithValues,
overrideAttributesDescribedWithoutValues
} = {}
) {
const describingAttributesWithValues = resolveAttributeList(
DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES,
overrideAttributesDescribedWithValues
);

const describingAttributesWithoutValues = resolveAttributeList(
DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES,
overrideAttributesDescribedWithoutValues
);

const { method, url, params } = requestOptions || {};

return describeRequestData(
{ method, url, params },
describingAttributesWithValues,
describingAttributesWithoutValues
);
}

/**
* Creates string that represents response data.
* Should allow to identify request, which was the source for this response.
* @param response - axios response object
* @returns {string}
* Creates a string that represents request data from a response.
* Helps to identify the request that was the source of the response.
*
* @param {Object} response - Axios response object.
* @param {Object} [options] - Options for describing attributes.
* @param {Array<string>} [options.overrideAttributesDescribedWithValues]
* @param {Array<string>} [options.overrideAttributesDescribedWithoutValues]
* @returns {string} A string representation of the request data.
*/
exports.describeRequestFromResponse = function (response) {
let methodUppercase = undefined;
let url = undefined;
let guid = undefined;
const responseConfig = response.config;
function describeRequestFromResponse(
response,
{
overrideAttributesDescribedWithValues,
overrideAttributesDescribedWithoutValues
} = {}
) {
let method;
let url;
let params;
const responseConfig = response?.config;

const describingAttributesWithValues = resolveAttributeList(
DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES,
overrideAttributesDescribedWithValues
);

const describingAttributesWithoutValues = resolveAttributeList(
DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES,
overrideAttributesDescribedWithoutValues
);

if (responseConfig) {
// Uppercase is used to allow finding logs corresponding to the same request - response pair,
// by matching identical options' descriptions.
methodUppercase = responseConfig.method.toUpperCase();
method = responseConfig.method;
url = responseConfig.url;
guid = responseConfig.params?.[sfParams.paramsNames.SF_REQUEST_GUID];
params = responseConfig.params;
}

return describeRequestData(
{ method, url, params },
describingAttributesWithValues,
describingAttributesWithoutValues
);
}

/**
* Constructs a string representation of request data.
*
* @param {Object} requestData - Object containing the method, url, and parameters.
* @param {string} requestData.method - HTTP method.
* @param {string} requestData.url - Request URL.
* @param {Object} [requestData.params] - Additional query parameters.
* @param {Array<string>} attributesWithValues - Attributes to describe with values.
* @param {Array<string>} attributesWithoutValues - Attributes to describe without values.
* @returns {string} A string describing the request data.
*/
function describeRequestData(
{ method, url, params } = {},
attributesWithValues,
attributesWithoutValues
) {
const requestObject = {
// Ensure consistent casing for methods to match request-response pairs in logs.
method: method?.toUpperCase(),
...constructURLData(url, params),
};

return LoggingUtil.describeAttributes(
requestObject,
attributesWithValues,
attributesWithoutValues
);
}

/**
* Constructs an object representing URL data including the base URL, path, and query parameters.
*
* @param {string} url - The full URL.
* @param {Object} [params] - Additional query parameters.
* @returns {Object} Contains baseUrl, path, and merged query parameters.
*/
function constructURLData(url, params = {}) {
if (!url) {
return { baseUrl: undefined, path: undefined, queryParams: {} };
}
return exports.describeRequestData({ method: methodUppercase, url: url, guid: guid });
};

exports.describeRequestData = function ({ method, url, guid } = {}) {
const guidDescription = guid ? `, guid: ${guid}` : '';
return `[method: ${method}, url: ${url}` + guidDescription + ']';
};
const urlObj = new URL(url);
const queryParams = { ...params };

urlObj.searchParams.forEach((value, key) => {
queryParams[key] = value;
});

const baseUrl = `${urlObj.protocol}//${urlObj.hostname}${urlObj.port ? `:${urlObj.port}` : ''}`;

return {
baseUrl: baseUrl,
path: urlObj.pathname,
...queryParams,
};
}

/**
* @param {string} url - The URL to describe.
* @param {Object} [options] - Options for describing attributes.
* @param {Array<string>} [options.overrideAttributesDescribedWithValues]
* @param {Array<string>} [options.overrideAttributesDescribedWithoutValues]
* @returns {string} A string describing the URL.
*/
function describeURL(
url,
{
overrideAttributesDescribedWithValues,
overrideAttributesDescribedWithoutValues
} = {}
) {
const describingAttributesWithValues = resolveAttributeList(
DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES,
overrideAttributesDescribedWithValues
);

const describingAttributesWithoutValues = resolveAttributeList(
DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES,
overrideAttributesDescribedWithoutValues
);

const urlData = constructURLData(url);

return LoggingUtil.describeAttributes(
urlData,
describingAttributesWithValues,
describingAttributesWithoutValues
);
}

exports.DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES = DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITH_VALUES;
exports.DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES = DEFAULT_ATTRIBUTES_DESCRIBING_REQUEST_WITHOUT_VALUES;

exports.describeRequestFromOptions = describeRequestFromOptions;
exports.describeRequestFromResponse = describeRequestFromResponse;
exports.describeURL = describeURL;
Loading

0 comments on commit 7a594c5

Please sign in to comment.