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.

Sign up for GitHub

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

Jump to bottom

Support HTTP_TIMEOUT environment variable #116

Merged
merged 4 commits into from
Mar 13, 2024
Merged

Support HTTP_TIMEOUT environment variable #116

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a25d709
Fix timeout error messaging
camden11 Mar 11, 2024
70475d0
Support HTTP_TIMEOUT environment variable
camden11 Mar 11, 2024
ed89a1c
fix tests
camden11 Mar 11, 2024
6484c6d
use isSpecifiedError for timeouts
camden11 Mar 12, 2024
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
32 changes: 26 additions & 6 deletions config/config_DEPRECATED.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,9 @@ function getConfigVariablesFromEnv() {
personalAccessKey: env[ENVIRONMENT_VARIABLES.HUBSPOT_PERSONAL_ACCESS_KEY],
portalId: parseInt(env[ENVIRONMENT_VARIABLES.HUBSPOT_PORTAL_ID] || '', 10),
refreshToken: env[ENVIRONMENT_VARIABLES.HUBSPOT_REFRESH_TOKEN],
httpTimeout: env[ENVIRONMENT_VARIABLES.HTTP_TIMEOUT]
? parseInt(env[ENVIRONMENT_VARIABLES.HTTP_TIMEOUT] || '')
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this part necessary? env[ENVIRONMENT_VARIABLES.HTTP_TIMEOUT] || ''

Since env[ENVIRONMENT_VARIABLES.HTTP_TIMEOUT] would already have to resolve to true for this section of the ternary to run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh good catch, that was left over from before I added the ternary (and would end with httpTimeout being NaN if not provided)

: undefined,
env: getValidEnv(
env[ENVIRONMENT_VARIABLES.HUBSPOT_ENVIRONMENT] as Environment
),
Expand All @@ -745,8 +748,9 @@ function getConfigVariablesFromEnv() {
function generatePersonalAccessKeyConfig(
portalId: number,
personalAccessKey: string,
env: Environment
): { portals: Array<CLIAccount_DEPRECATED> } {
env: Environment,
httpTimeout?: number
): { portals: Array<CLIAccount_DEPRECATED>; httpTimeout?: number } {
return {
portals: [
{
Expand All @@ -756,6 +760,7 @@ function generatePersonalAccessKeyConfig(
env,
},
],
httpTimeout,
};
}

Expand All @@ -765,8 +770,9 @@ function generateOauthConfig(
clientSecret: string,
refreshToken: string,
scopes: Array<string>,
env: Environment
): { portals: Array<OAuthAccount_DEPRECATED> } {
env: Environment,
httpTimeout?: number
): { portals: Array<OAuthAccount_DEPRECATED>; httpTimeout?: number } {
return {
portals: [
{
Expand All @@ -783,6 +789,7 @@ function generateOauthConfig(
env,
},
],
httpTimeout,
};
}

Expand Down Expand Up @@ -816,6 +823,7 @@ export function loadConfigFromEnvironment({
portalId,
refreshToken,
env,
httpTimeout,
} = getConfigVariablesFromEnv();
const unableToLoadEnvConfigError =
'Unable to load config from environment variables.';
Expand All @@ -825,16 +833,28 @@ export function loadConfigFromEnvironment({
return;
}

if (httpTimeout && httpTimeout < MIN_HTTP_TIMEOUT) {
throw new Error(
`The HTTP timeout value ${httpTimeout} is invalid. The value must be a number greater than ${MIN_HTTP_TIMEOUT}.`
);
}

if (personalAccessKey) {
return generatePersonalAccessKeyConfig(portalId, personalAccessKey, env);
return generatePersonalAccessKeyConfig(
portalId,
personalAccessKey,
env,
httpTimeout
);
} else if (clientId && clientSecret && refreshToken) {
return generateOauthConfig(
portalId,
clientId,
clientSecret,
refreshToken,
OAUTH_SCOPES.map(scope => scope.value),
env
env,
httpTimeout
);
} else if (apiKey) {
return generateApiKeyConfig(portalId, apiKey, env);
Expand Down
1 change: 1 addition & 0 deletions constants/environments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ export const ENVIRONMENT_VARIABLES = {
HUBSPOT_PORTAL_ID: 'HUBSPOT_PORTAL_ID',
HUBSPOT_REFRESH_TOKEN: 'HUBSPOT_REFRESH_TOKEN',
HUBSPOT_ENVIRONMENT: 'HUBSPOT_ENVIRONMENT',
HTTP_TIMEOUT: 'HTTP_TIMEOUT',
} as const;
9 changes: 9 additions & 0 deletions http/__tests__/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ describe('http/index', () => {
params: {
portalId: 123,
},
transitional: {
clarifyTimeoutError: true,
},
});
});
it('adds authorization header when using a user token', async () => {
Expand Down Expand Up @@ -182,6 +185,9 @@ describe('http/index', () => {
params: {
portalId: 123,
},
transitional: {
clarifyTimeoutError: true,
},
});
});

Expand Down Expand Up @@ -215,6 +221,9 @@ describe('http/index', () => {
portalId: 123,
hapikey: 'abc',
},
transitional: {
clarifyTimeoutError: true,
},
});
});
});
Expand Down
5 changes: 5 additions & 0 deletions http/getAxiosConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ export const DEFAULT_USER_AGENT_HEADERS = {
'User-Agent': `HubSpot Local Dev Lib/${version}`,
};

const DEFAULT_TRANSITIONAL = {
clarifyTimeoutError: true,
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This tells axios to use ETIMEDOUT as the code for timeout errors


export function getAxiosConfig(
options: AxiosConfigOptions
): AxiosRequestConfig {
Expand All @@ -26,6 +30,7 @@ export function getAxiosConfig(
...(headers || {}),
},
timeout: httpTimeout || 15000,
transitional: DEFAULT_TRANSITIONAL,
...rest,
};
}
40 changes: 18 additions & 22 deletions lib/fileMapper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ async function writeFileMapperNode(
}

function isTimeout(err: BaseError): boolean {
return !!err && (err.status === 408 || err.code === 'ESOCKETTIMEDOUT');
return !!err && (err.status === 408 || err.code === 'ETIMEDOUT');
Copy link
Contributor

Choose a reason for hiding this comment

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

Would isSpecifiedError help here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah maybe we should update this to be able to check for code

}

async function downloadFile(
Expand Down Expand Up @@ -332,22 +332,13 @@ export async function fetchFolderFromApi(
src,
});
}
try {
const srcPath = isRoot ? '@root' : src;
const queryValues = getFileMapperQueryValues(mode, options);
const node = isHubspot
? await downloadDefault(accountId, srcPath, queryValues)
: await download(accountId, srcPath, queryValues);
logger.log(i18n(`${i18nKey}.folderFetch`, { src, accountId }));
return node;
} catch (err) {
const error = err as BaseError;
if (isHubspot && isTimeout(error)) {
throwErrorWithMessage(`${i18nKey}.errors.assetTimeout`, {}, error);
} else {
throwError(error);
}
}
const srcPath = isRoot ? '@root' : src;
const queryValues = getFileMapperQueryValues(mode, options);
const node = isHubspot
? await downloadDefault(accountId, srcPath, queryValues)
: await download(accountId, srcPath, queryValues);
logger.log(i18n(`${i18nKey}.folderFetch`, { src, accountId }));
return node;
}

async function downloadFolder(
Expand Down Expand Up @@ -401,11 +392,16 @@ async function downloadFolder(
throwErrorWithMessage(`${i18nKey}.errors.incompleteFetch`, { src });
}
} catch (err) {
throwErrorWithMessage(
`${i18nKey}.errors.failedToFetchFolder`,
{ src, dest: destPath },
err as AxiosError
);
const error = err as AxiosError;
if (isTimeout(error)) {
throwErrorWithMessage(`${i18nKey}.errors.assetTimeout`, {}, error);
} else {
throwErrorWithMessage(
`${i18nKey}.errors.failedToFetchFolder`,
{ src, dest: destPath },
err as AxiosError
);
}
}
}

Expand Down
Loading