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

feat(cli): Take request headers, params, and body into account when saving responses #3098

Merged
merged 16 commits into from
Dec 5, 2024
Merged
135 changes: 99 additions & 36 deletions packages/cli/lib/services/response-saver.service.ts
Original file line number Diff line number Diff line change
@@ -1,38 +1,37 @@
import fs from 'node:fs';
import path from 'node:path';
import type { AxiosResponse } from 'axios';
import crypto from 'node:crypto';
import type { AxiosRequestConfig, AxiosResponse } from 'axios';
import type { Connection } from '@nangohq/shared';
import type { Metadata } from '@nangohq/types';

const FILTER_HEADERS = ['authorization', 'user-agent', 'nango-proxy-user-agent', 'accept-encoding', 'retries', 'host', 'connection-id', 'provider-config-key'];

interface ConfigIdentity {
method: string;
endpoint: string;
requestIdentityHash: string;
requestIdentity: unknown[];
}

interface CachedRequest {
requestIdentityHash: string;
requestIdentity: unknown[];
response: unknown;
}

export function ensureDirectoryExists(directoryName: string): void {
if (!fs.existsSync(directoryName)) {
fs.mkdirSync(directoryName, { recursive: true });
}
}

function saveResponse<T>({
directoryName,
data,
customFilePath,
concatenateIfExists
}: {
directoryName: string;
data: T | T[];
customFilePath: string;
concatenateIfExists: boolean;
}): void {
function saveResponse<T>({ directoryName, data, customFilePath }: { directoryName: string; data: T | T[]; customFilePath: string }): void {
ensureDirectoryExists(`${directoryName}/mocks`);

const filePath = path.join(directoryName, customFilePath);
ensureDirectoryExists(path.dirname(filePath));

if (fs.existsSync(filePath)) {
const existingData = JSON.parse(fs.readFileSync(filePath, 'utf8'));
if (concatenateIfExists && Array.isArray(existingData) && Array.isArray(data)) {
data = data.concat(existingData);
}
}

fs.writeFileSync(filePath, JSON.stringify(data, null, 2));
}

Expand All @@ -51,7 +50,6 @@ export function onAxiosRequestFulfilled({
return response;
}
const directoryName = `${process.env['NANGO_MOCKS_RESPONSE_DIRECTORY'] ?? ''}${providerConfigKey}`;
const method = response.config.method?.toLowerCase() || 'get';

if (response.request.path.includes(`/connection/${connectionId}?provider_config_key=${providerConfigKey}`)) {
const connection = response.data as Connection;
Expand All @@ -60,35 +58,100 @@ export function onAxiosRequestFulfilled({
saveResponse<Pick<Connection, 'metadata' | 'connection_config'>>({
directoryName,
data: { metadata: connection.metadata as Metadata, connection_config: connection.connection_config },
customFilePath: 'mocks/nango/getConnection.json',
concatenateIfExists: false
customFilePath: 'mocks/nango/getConnection.json'
});

saveResponse<Metadata>({
directoryName,
data: connection.metadata as Metadata,
customFilePath: 'mocks/nango/getMetadata.json',
concatenateIfExists: false
customFilePath: 'mocks/nango/getMetadata.json'
});

return response;
}

const [pathname, params] = response.request.path.split('?');
const strippedPath = pathname.replace('/', '');
const requestIdentity = computeConfigIdentity(response.config);

let concatenateIfExists = false;

if (params && params.includes('page')) {
concatenateIfExists = true;
}

saveResponse<AxiosResponse>({
saveResponse<CachedRequest>({
directoryName,
data: response.data,
customFilePath: `mocks/nango/${method}/${strippedPath}/${syncName}.json`,
concatenateIfExists
data: {
...requestIdentity,
response: response.data
},
customFilePath: `mocks/nango/${requestIdentity.method}/${requestIdentity.endpoint}/${syncName}/${requestIdentity.requestIdentityHash}.json`
});

return response;
}

function computeConfigIdentity(config: AxiosRequestConfig): ConfigIdentity {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit confused by this, what does config identity means?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's basically a computed identity for a request config for the case of our integration tests in integration-templates. We'd consider two requests identical if the JSON.stringy of two ConfigIdentity objects is the same.

Copy link
Collaborator

Choose a reason for hiding this comment

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

mmh yeah okay, in that case it's not really just an identity it's just an object that can be used as an identity, got confused by that and config is generic but I guess it maps axios config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, Config is kind of overloaded for sure. I'm dumping the object only so that it's easier for us to debug when identities don't match - it can be tricky with just a hash to know why something changed.

const method = config.method?.toLowerCase() || 'get';
const params = sortEntries(Object.entries(config.params || {}));

const url = new URL(config.url!);
const endpoint = url.pathname.replace(/^\/proxy\//, '');

const requestIdentity: [string, unknown][] = [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TBonnin @khaliqgant I had to add some new logic in here to compute endpoint as best I could and use that since in the test env we don't have a full URL to match against.

['method', method],
['endpoint', endpoint],
['params', params]
];

const dataIdentity = computeDataIdentity(config);
if (dataIdentity) {
requestIdentity.push(['data', dataIdentity]);
}

if (config.headers !== undefined) {
const filteredHeaders = Object.entries(config.headers).filter(([key]) => !FILTER_HEADERS.includes(key.toLowerCase()));
sortEntries(filteredHeaders);
requestIdentity.push([`headers`, filteredHeaders]);
}

// sort by key so we have a consistent hash
sortEntries(requestIdentity);

const requestIdentityHash = crypto.createHash('sha1').update(JSON.stringify(requestIdentity)).digest('hex');

return {
method,
endpoint,
requestIdentityHash,
requestIdentity
};
}

function sortEntries(entries: [string, unknown][]): [string, unknown][] {
return entries.sort((a, b) => (a[0] < b[0] ? -1 : a[0] > b[0] ? 1 : 0));
}

function computeDataIdentity(config: AxiosRequestConfig): string | undefined {
const data = config.data;

if (!data) {
return undefined;
}

let dataString = '';
if (typeof data === 'string') {
dataString = data;
} else if (Buffer.isBuffer(data)) {
dataString = data.toString('base64');
} else {
try {
dataString = JSON.stringify(data);
} catch (e) {
if (e instanceof Error) {
throw new Error(`Unable to compute request identity: ${e.message}`);
} else {
throw new Error('Unable to compute request identity');
}
}
}

if (dataString.length > 1000) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not always hashing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's easier to spot why things changed in the recorded payloads if you dump the literal payload, rather than just a hash, but when the payloads get extremely large it's not really reasonable to store them anymore.

return 'sha1:' + crypto.createHash('sha1').update(dataString).digest('hex');
} else {
return dataString;
}
}
Loading