fix: apply override_params to FormData request bodies#1242
fix: apply override_params to FormData request bodies#1242AgileEduLabs wants to merge 1 commit intoPortkey-AI:mainfrom
Conversation
|
Summary By MatterAI
🔄 What ChangedThis PR adds support for applying 🔍 Impact of the ChangeThis change ensures consistent behavior across all request body types when using the 📁 Total Files Changed
🧪 Test AddedN/A - No tests were included in this PR based on the provided JSON data. 🔒 Security VulnerabilitiesN/A - No security vulnerabilities were identified in this change. Tip Quality Recommendations
Sequence DiagramsequenceDiagram
participant Client
participant transformToProviderRequest
participant FormData
Client->>transformToProviderRequest: Call with requestBody (FormData) and providerOptions
Note over transformToProviderRequest: Check if requestBody is FormData
alt requestBody is FormData
alt providerOptions.overrideParams is undefined
transformToProviderRequest-->>Client: Return original FormData unchanged
else providerOptions.overrideParams exists
loop For each [key, value] in overrideParams
transformToProviderRequest->>FormData: delete(key)
Note over transformToProviderRequest: Convert value to array if not already
loop For each val in values array
transformToProviderRequest->>FormData: append(key, val)
end
end
transformToProviderRequest-->>Client: Return modified FormData
end
else requestBody is ArrayBuffer
transformToProviderRequest-->>Client: Return original ArrayBuffer unchanged
end
|
![matterai-app[bot]](https://avatars.githubusercontent.com/in/474549?s=60&v=4){kind=small}
| if (requestBody instanceof FormData) { | ||
| if (typeof providerOptions.overrideParams === 'undefined') { | ||
| return requestBody; | ||
| } | ||
|
|
||
| for (const [k, v] of Object.entries(providerOptions.overrideParams)) { | ||
| requestBody.delete(k); // If the key already exists, delete it first so we always overwrite. | ||
|
|
||
| // Value can be string, Blob, File, or an array of those. | ||
| const values = Array.isArray(v) ? v : [v]; | ||
| for (const val of values) { | ||
| requestBody.append(k, val); | ||
| } | ||
| } | ||
|
|
||
| return requestBody; | ||
|
|
||
| } else if (requestBody instanceof ArrayBuffer) |
There was a problem hiding this comment.
🛠️ Code Refactor
Issue: The code doesn't handle potential errors that might occur during FormData manipulation.
Fix: Add try-catch block to handle potential exceptions during FormData operations.
Impact: Improves error handling and prevents unexpected crashes if FormData operations fail.
| if (requestBody instanceof FormData) { | |
| if (typeof providerOptions.overrideParams === 'undefined') { | |
| return requestBody; | |
| } | |
| for (const [k, v] of Object.entries(providerOptions.overrideParams)) { | |
| requestBody.delete(k); // If the key already exists, delete it first so we always overwrite. | |
| // Value can be string, Blob, File, or an array of those. | |
| const values = Array.isArray(v) ? v : [v]; | |
| for (const val of values) { | |
| requestBody.append(k, val); | |
| } | |
| } | |
| return requestBody; | |
| } else if (requestBody instanceof ArrayBuffer) | |
| if (requestBody instanceof FormData) { | |
| if (typeof providerOptions.overrideParams === 'undefined') { | |
| return requestBody; | |
| } | |
| try { | |
| for (const [k, v] of Object.entries(providerOptions.overrideParams)) { | |
| requestBody.delete(k); // If the key already exists, delete it first so we always overwrite. | |
| // Value can be string, Blob, File, or an array of those. | |
| const values = Array.isArray(v) ? v : [v]; | |
| for (const val of values) { | |
| requestBody.append(k, val); | |
| } | |
| } | |
| return requestBody; | |
| } catch (error) { | |
| console.error('Error applying overrideParams to FormData:', error); | |
| return requestBody; // Return original FormData if operation fails | |
| } | |
| } else if (requestBody instanceof ArrayBuffer) |
| // Value can be string, Blob, File, or an array of those. | ||
| const values = Array.isArray(v) ? v : [v]; | ||
| for (const val of values) { | ||
| requestBody.append(k, val); | ||
| } |
There was a problem hiding this comment.
🛠️ Code Refactor
Issue: The code doesn't validate that the values in overrideParams are compatible with FormData.append().
Fix: Add type checking to ensure values are compatible with FormData.append() which accepts string, Blob, or File.
Impact: Prevents runtime errors when incompatible types are provided in overrideParams.
| // Value can be string, Blob, File, or an array of those. | |
| const values = Array.isArray(v) ? v : [v]; | |
| for (const val of values) { | |
| requestBody.append(k, val); | |
| } | |
| // Value can be string, Blob, File, or an array of those. | |
| const values = Array.isArray(v) ? v : [v]; | |
| for (const val of values) { | |
| if (val instanceof Blob || typeof val === 'string') { | |
| requestBody.append(k, val); | |
| } else { | |
| // For other types, convert to string | |
| requestBody.append(k, String(val)); | |
| } | |
| } |
|
@AgileEduLabs I think we need to take a more nuanced approach with regards to the tts and stt routes unification, though we currently support loading and transforming multipart form data for these routes, It is not scalable, I want to verify if we can transform these on the fly with stream transforms, similar to how we are doing for file uploads |
3 participants
Description
Transform the javascript FormData object based on the supplied override_params. Modified transformToProviderRequest.ts on line 235:
Motivation
Currently
portkey.audio.transcriptions.createdoes not respect theoverride_paramsof the gateway config object. For example, if we change the model name inoverride_params, we would expect it to use the model name as specified in the config object. However, this values in the config object are ignored.If override_params cannot modify the model name on the gateway, then the gateway features are essentially useless. You won't be able to do any sort of meaningful fallback nor loadbalancing, because each provider has their own model name.
The source of the problem lies in transformToProviderRequest.ts on line 235. If body is FormData, then override parameters are not applied and instead the function returns early with the requestBody unmodified:
At first glance this makes sense because you cannot apply a simple spread operator to FormData nor ArrayBuffer, so we just ignore the override_params and return early.
However, the more elegant solution is to transform the javascript FormData object based on the override_params. Since requestBody is already a JavaScript FormData object, the computational cost of overriding the keys is negligible. This is in keeping with the spirit of the gateway's lightweight nature:
This pull request is to fix the issue as outlined in #1217
Type of Change
How Has This Been Tested?
Screenshots (if applicable)
Checklist
Related Issues