Je/connect proxy support #15495
Je/connect proxy support #15495
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces a new proxy request capability in the SDK. The Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant B as BackendClient
participant M as makeConnectRequest
participant S as Proxy Service
C->>B: makeProxyRequest(url, query, opts)
B->>B: Encode URL (base64)\nModify Headers (add prefix)
B->>M: Call makeConnectRequest with constructed options
M-->>B: Return response
B-->>C: Return proxy response
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/sdk/src/server/index.ts(3 hunks)packages/sdk/src/shared/index.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Lint Code Base
packages/sdk/src/server/index.ts
[failure] 7-7:
Missing trailing comma
[failure] 120-120:
Unexpected any. Specify a different type
[failure] 396-396:
Unexpected any. Specify a different type
[failure] 396-396:
Unexpected any. Specify a different type
[failure] 398-398:
Expected indentation of 6 spaces but found 4
[failure] 399-399:
Expected indentation of 6 spaces but found 4
🪛 ESLint
packages/sdk/src/server/index.ts
[error] 7-8: Missing trailing comma.
(comma-dangle)
[error] 120-120: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 396-396: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 396-396: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 398-398: Expected indentation of 6 spaces but found 4.
(indent)
[error] 399-399: Expected indentation of 6 spaces but found 4.
(indent)
🪛 GitHub Actions: Pull Request Checks
packages/sdk/src/server/index.ts
[error] 7-7: Missing trailing comma
🔇 Additional comments (1)
packages/sdk/src/shared/index.ts (1)
999-999: LGTM!The header handling change in
makeAuthorizedRequestis correct and maintains proper type safety.
packages/sdk/src/server/index.ts
Outdated
| @@ -4,7 +4,7 @@ | |||
|
|
|||
| import * as oauth from "oauth4webapi"; | |||
| import { | |||
| Account, BaseClient, type AppInfo, type ConnectTokenResponse, | |||
| Account, BaseClient, type AppInfo, type ConnectTokenResponse, type RequestOptions | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add missing trailing comma in import statement.
Add a trailing comma after RequestOptions to comply with the project's linting rules.
- Account, BaseClient, type AppInfo, type ConnectTokenResponse, type RequestOptions
+ Account, BaseClient, type AppInfo, type ConnectTokenResponse, type RequestOptions,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Account, BaseClient, type AppInfo, type ConnectTokenResponse, type RequestOptions | |
| Account, BaseClient, type AppInfo, type ConnectTokenResponse, type RequestOptions, |
🧰 Tools
🪛 GitHub Check: Lint Code Base
[failure] 7-7:
Missing trailing comma
🪛 ESLint
[error] 7-8: Missing trailing comma.
(comma-dangle)
🪛 GitHub Actions: Pull Request Checks
[error] 7-7: Missing trailing comma
packages/sdk/src/server/index.ts
Outdated
| /** | ||
| * Parameters for the retrieval of an account from the Connect API | ||
| */ | ||
| export type MakeProxyRequestOpts = { | ||
| /** | ||
| * Whether to retrieve the account's credentials or not. | ||
| */ | ||
| method: string; | ||
| headers?: Record<string, string>; | ||
| //headers: Record<string, string>; | ||
| body?: any | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix type definition issues.
The MakeProxyRequestOpts type definition has several issues:
- The JSDoc comment is incorrect (copied from GetAccountByIdOpts)
- Contains commented out code
- Uses
anytype which should be properly typed
/**
- * Parameters for the retrieval of an account from the Connect API
+ * Options for making a proxy request through the Connect API
*/
export type MakeProxyRequestOpts = {
/**
- * Whether to retrieve the account's credentials or not.
+ * The HTTP method to use for the proxy request
*/
method: string;
+ /**
+ * Headers to include in the proxy request
+ */
headers?: Record<string, string>;
- //headers: Record<string, string>;
+ /**
+ * The request body
+ */
- body?: any
+ body?: Record<string, unknown> | string | FormData | URLSearchParams | null;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Parameters for the retrieval of an account from the Connect API | |
| */ | |
| export type MakeProxyRequestOpts = { | |
| /** | |
| * Whether to retrieve the account's credentials or not. | |
| */ | |
| method: string; | |
| headers?: Record<string, string>; | |
| //headers: Record<string, string>; | |
| body?: any | |
| }; | |
| /** | |
| * Options for making a proxy request through the Connect API | |
| */ | |
| export type MakeProxyRequestOpts = { | |
| /** | |
| * The HTTP method to use for the proxy request | |
| */ | |
| method: string; | |
| /** | |
| * Headers to include in the proxy request | |
| */ | |
| headers?: Record<string, string>; | |
| /** | |
| * The request body | |
| */ | |
| body?: Record<string, unknown> | string | FormData | URLSearchParams | null; | |
| }; |
🧰 Tools
🪛 GitHub Check: Lint Code Base
[failure] 120-120:
Unexpected any. Specify a different type
🪛 ESLint
[error] 120-120: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
packages/sdk/src/server/index.ts
Outdated
| public makeProxyRequest(url: string, query: any, opts: MakeProxyRequestOpts): Promise<any> { | ||
| const url64 = btoa(url).replace(/\+/g, "-") | ||
| .replace(/\//g, "_") | ||
| .replace(/=+$/, ""); | ||
|
|
||
| const headers = opts.headers || {}; | ||
|
|
||
| const newHeaders = Object.keys(headers).reduce<{ [key: string]: string }>((acc, key) => { | ||
| acc[`x-pd-proxy-${key}`] = headers[key]; | ||
| return acc; | ||
| }, {}); | ||
|
|
||
| const newOpts: RequestOptions = { | ||
| method: opts.method, | ||
| headers: newHeaders, | ||
| params: query, | ||
| } | ||
|
|
||
| if (opts.body) { | ||
| newOpts.body = opts.body | ||
| } | ||
|
|
||
| return this.makeConnectRequest(`/proxy/${url64}`, newOpts); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and error handling in proxy request implementation.
The implementation should:
- Use proper types instead of
any - Add input validation
- Handle potential base64 encoding errors
- Fix indentation
- public makeProxyRequest(url: string, query: any, opts: MakeProxyRequestOpts): Promise<any> {
+ public makeProxyRequest(
+ url: string,
+ query: Record<string, string | number | boolean | null>,
+ opts: MakeProxyRequestOpts
+ ): Promise<unknown> {
+ if (!url?.trim()) {
+ throw new Error("URL is required");
+ }
+ let url64: string;
+ try {
url64 = btoa(url).replace(/\+/g, "-")
- .replace(/\//g, "_")
- .replace(/=+$/, "");
+ .replace(/\//g, "_")
+ .replace(/=+$/, "");
+ } catch (error) {
+ throw new Error(`Failed to encode URL: ${error.message}`);
+ }
const headers = opts.headers || {};
const newHeaders = Object.keys(headers).reduce<{ [key: string]: string }>((acc, key) => {
acc[`x-pd-proxy-${key}`] = headers[key];
return acc;
}, {});
const newOpts: RequestOptions = {
method: opts.method,
headers: newHeaders,
params: query,
}
if (opts.body) {
newOpts.body = opts.body
}
return this.makeConnectRequest(`/proxy/${url64}`, newOpts);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public makeProxyRequest(url: string, query: any, opts: MakeProxyRequestOpts): Promise<any> { | |
| const url64 = btoa(url).replace(/\+/g, "-") | |
| .replace(/\//g, "_") | |
| .replace(/=+$/, ""); | |
| const headers = opts.headers || {}; | |
| const newHeaders = Object.keys(headers).reduce<{ [key: string]: string }>((acc, key) => { | |
| acc[`x-pd-proxy-${key}`] = headers[key]; | |
| return acc; | |
| }, {}); | |
| const newOpts: RequestOptions = { | |
| method: opts.method, | |
| headers: newHeaders, | |
| params: query, | |
| } | |
| if (opts.body) { | |
| newOpts.body = opts.body | |
| } | |
| return this.makeConnectRequest(`/proxy/${url64}`, newOpts); | |
| } | |
| public makeProxyRequest( | |
| url: string, | |
| query: Record<string, string | number | boolean | null>, | |
| opts: MakeProxyRequestOpts | |
| ): Promise<unknown> { | |
| if (!url?.trim()) { | |
| throw new Error("URL is required"); | |
| } | |
| let url64: string; | |
| try { | |
| url64 = btoa(url).replace(/\+/g, "-") | |
| .replace(/\//g, "_") | |
| .replace(/=+$/, ""); | |
| } catch (error: any) { | |
| throw new Error(`Failed to encode URL: ${error.message}`); | |
| } | |
| const headers = opts.headers || {}; | |
| const newHeaders = Object.keys(headers).reduce<{ [key: string]: string }>((acc, key) => { | |
| acc[`x-pd-proxy-${key}`] = headers[key]; | |
| return acc; | |
| }, {}); | |
| const newOpts: RequestOptions = { | |
| method: opts.method, | |
| headers: newHeaders, | |
| params: query, | |
| } | |
| if (opts.body) { | |
| newOpts.body = opts.body; | |
| } | |
| return this.makeConnectRequest(`/proxy/${url64}`, newOpts); | |
| } |
🧰 Tools
🪛 GitHub Check: Lint Code Base
[failure] 396-396:
Unexpected any. Specify a different type
[failure] 396-396:
Unexpected any. Specify a different type
[failure] 398-398:
Expected indentation of 6 spaces but found 4
[failure] 399-399:
Expected indentation of 6 spaces but found 4
🪛 ESLint
[error] 396-396: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 396-396: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 398-398: Expected indentation of 6 spaces but found 4.
(indent)
[error] 399-399: Expected indentation of 6 spaces but found 4.
(indent)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/sdk/src/server/index.ts (3)
113-118: Strengthen type safety for required search parameters.The
searchParamstype should explicitly define required fields for better type safety and developer experience.- searchParams: Record<string, string>; + searchParams: { + external_user_id: string; + account_id: string; + [key: string]: string; + };
123-136: Improve body type to support common request payload types.The
bodytype is too restrictive. Consider supporting other common payload types like JSON objects, FormData, etc.- body?: string; + body?: string | Record<string, unknown> | FormData | URLSearchParams;
420-424: Improve JSDoc documentation for makeProxyRequest method.The current documentation lacks:
- Parameter descriptions
- Return type details
- Usage example
/** * Makes a proxy request to the target app API with the specified query parameters and options. + * + * @param proxyOptions - Options containing required search parameters (external_user_id and account_id) + * @param targetRequest - Target request details including URL and request options + * @returns A promise resolving to the response from the downstream service + * + * @example + * ```typescript + * const response = await client.makeProxyRequest( + * { + * searchParams: { + * external_user_id: "user123", + * account_id: "acc456" + * } + * }, + * { + * url: "https://api.example.com/data", + * options: { + * method: "GET", + * headers: { "Accept": "application/json" } + * } + * } + * ); + * ``` */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/sdk/src/server/index.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: pnpm publish
🔇 Additional comments (1)
packages/sdk/src/server/index.ts (1)
7-7: Add missing trailing comma in import statement.Add a trailing comma after
RequestOptionsto comply with the project's linting rules.- Account, BaseClient, type AppInfo, type ConnectTokenResponse, type RequestOptions + Account, BaseClient, type AppInfo, type ConnectTokenResponse, type RequestOptions,
| public makeProxyRequest(proxyOptions: ProxyApiOpts, targetRequest: ProxyTargetApiRequest): Promise<string> { | ||
| const url64 = btoa(targetRequest.url).replace(/\+/g, "-") | ||
| .replace(/\//g, "_") | ||
| .replace(/=+$/, ""); | ||
|
|
||
| const headers = targetRequest.options.headers || {}; | ||
|
|
||
| const newHeaders = Object.keys(headers).reduce<{ [key: string]: string }>((acc, key) => { | ||
| acc[`x-pd-proxy-${key}`] = headers[key]; | ||
| return acc; | ||
| }, {}); | ||
|
|
||
| const newOpts: RequestOptions = { | ||
| method: targetRequest.options.method, | ||
| headers: newHeaders, | ||
| params: proxyOptions.searchParams, | ||
| } | ||
|
|
||
| if (targetRequest.options.body) { | ||
| newOpts.body = targetRequest.options.body | ||
| } | ||
|
|
||
| return this.makeConnectRequest(`/proxy/${url64}`, newOpts); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and input validation in proxy request implementation.
The implementation needs:
- URL validation
- Error handling for base64 encoding
- Validation for required search parameters
- Proper indentation
public makeProxyRequest(proxyOptions: ProxyApiOpts, targetRequest: ProxyTargetApiRequest): Promise<string> {
+ // Validate required search parameters
+ if (!proxyOptions.searchParams.external_user_id || !proxyOptions.searchParams.account_id) {
+ throw new Error("external_user_id and account_id are required in searchParams");
+ }
+
+ // Validate URL
+ if (!targetRequest.url?.trim()) {
+ throw new Error("URL is required");
+ }
+
+ let url64: string;
+ try {
url64 = btoa(targetRequest.url).replace(/\+/g, "-")
- .replace(/\//g, "_")
- .replace(/=+$/, "");
+ .replace(/\//g, "_")
+ .replace(/=+$/, "");
+ } catch (error) {
+ throw new Error(`Failed to encode URL: ${error.message}`);
+ }
const headers = targetRequest.options.headers || {};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public makeProxyRequest(proxyOptions: ProxyApiOpts, targetRequest: ProxyTargetApiRequest): Promise<string> { | |
| const url64 = btoa(targetRequest.url).replace(/\+/g, "-") | |
| .replace(/\//g, "_") | |
| .replace(/=+$/, ""); | |
| const headers = targetRequest.options.headers || {}; | |
| const newHeaders = Object.keys(headers).reduce<{ [key: string]: string }>((acc, key) => { | |
| acc[`x-pd-proxy-${key}`] = headers[key]; | |
| return acc; | |
| }, {}); | |
| const newOpts: RequestOptions = { | |
| method: targetRequest.options.method, | |
| headers: newHeaders, | |
| params: proxyOptions.searchParams, | |
| } | |
| if (targetRequest.options.body) { | |
| newOpts.body = targetRequest.options.body | |
| } | |
| return this.makeConnectRequest(`/proxy/${url64}`, newOpts); | |
| } | |
| public makeProxyRequest(proxyOptions: ProxyApiOpts, targetRequest: ProxyTargetApiRequest): Promise<string> { | |
| // Validate required search parameters | |
| if (!proxyOptions.searchParams.external_user_id || !proxyOptions.searchParams.account_id) { | |
| throw new Error("external_user_id and account_id are required in searchParams"); | |
| } | |
| // Validate URL | |
| if (!targetRequest.url?.trim()) { | |
| throw new Error("URL is required"); | |
| } | |
| let url64: string; | |
| try { | |
| url64 = btoa(targetRequest.url).replace(/\+/g, "-") | |
| .replace(/\//g, "_") | |
| .replace(/=+$/, ""); | |
| } catch (error) { | |
| throw new Error(`Failed to encode URL: ${error.message}`); | |
| } | |
| const headers = targetRequest.options.headers || {}; | |
| const newHeaders = Object.keys(headers).reduce<{ [key: string]: string }>((acc, key) => { | |
| acc[`x-pd-proxy-${key}`] = headers[key]; | |
| return acc; | |
| }, {}); | |
| const newOpts: RequestOptions = { | |
| method: targetRequest.options.method, | |
| headers: newHeaders, | |
| params: proxyOptions.searchParams, | |
| } | |
| if (targetRequest.options.body) { | |
| newOpts.body = targetRequest.options.body; | |
| } | |
| return this.makeConnectRequest(`/proxy/${url64}`, newOpts); | |
| } |
packages/sdk/src/server/index.ts
Outdated
| /** | ||
| * http method for the request | ||
| */ | ||
| method: string; |
There was a problem hiding this comment.
I think we should list all supported methods as a type like get | post | put | delete
| @@ -4,7 +4,7 @@ | |||
|
|
|||
There was a problem hiding this comment.
Missing version update and CHANGELOG
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/sdk/CHANGELOG.md(1 hunks)packages/sdk/package.json(1 hunks)packages/sdk/src/server/index.ts(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/sdk/package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: pnpm publish
🔇 Additional comments (2)
packages/sdk/src/server/index.ts (2)
7-7: Add missing trailing comma in import statement.
425-448: Improve error handling and input validation in proxy request implementation.
| export type ProxyApiOpts = { | ||
| /** | ||
| * Search parameters to be added to the proxy request. external_user_id and account_id are required. | ||
| */ | ||
| searchParams: Record<string, string>; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enforce required fields in ProxyApiOpts type.
The type definition should enforce the required fields mentioned in the JSDoc comment.
export type ProxyApiOpts = {
/**
* Search parameters to be added to the proxy request. external_user_id and account_id are required.
*/
- searchParams: Record<string, string>;
+ searchParams: {
+ external_user_id: string;
+ account_id: string;
+ [key: string]: string;
+ };
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type ProxyApiOpts = { | |
| /** | |
| * Search parameters to be added to the proxy request. external_user_id and account_id are required. | |
| */ | |
| searchParams: Record<string, string>; | |
| }; | |
| export type ProxyApiOpts = { | |
| /** | |
| * Search parameters to be added to the proxy request. external_user_id and account_id are required. | |
| */ | |
| searchParams: { | |
| external_user_id: string; | |
| account_id: string; | |
| [key: string]: string; | |
| }; | |
| }; |
| ## [1.3.3] - 2025-02-5 | ||
|
|
||
| ### Changed | ||
|
|
||
| - Add makeProxyRequest function to BaseClient | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance the changelog entry with more details.
The changelog entry should provide more context about the proxy request feature:
- Describe the purpose and benefits
- Document the new types added
- Include usage examples or migration guidance
## [1.3.3] - 2025-02-5
### Changed
-- Add makeProxyRequest function to BaseClient
+### Added
+
+- Added proxy request support to enable secure API calls through Pipedream's infrastructure
+ - New `makeProxyRequest` function in BaseClient for making proxy requests
+ - New types: `ProxyApiOpts`, `ProxyTargetApiOpts`, and `ProxyTargetApiRequest`
+
+Example usage:
+```typescript
+const response = await client.makeProxyRequest(
+ { searchParams: { external_user_id: "user123", account_id: "acc456" } },
+ {
+ url: "https://api.example.com/data",
+ options: {
+ method: "GET",
+ headers: { "Accept": "application/json" }
+ }
+ }
+);
+```
WHY
Summary by CodeRabbit
New Features
Bug Fixes
Deprecations