Skip to content

Commit

Permalink
[OpenAI & Core] Fix whisper responseFormat behavior (Azure#28148)
Browse files Browse the repository at this point in the history
### Packages impacted by this PR
@azure/openai & @azure/core-rest-pipeline

### Issues associated with this PR
Azure#28145

### Describe the problem that is addressed by this PR

The `responseFormat` parameter is included in a form data request. If
the customer does not specify it, the parameter is added to the form
data request with a value of undefined. However, the form data policy
attempts to treat this value as a File object.

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

To avoid sending an incorrect parameter, the fix is to not send it if it
is not set by the customer. Additionally, the core should verify that
the form data value is defined and is an object before proceeding to
treat it as such.

### Are there test cases added in this PR? _(If not, why?)_
Yes

### Provide a list of related PRs _(if any)_
N/A

### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
- [x] Added a changelog (if necessary)
  • Loading branch information
deyaaeldeen authored Jan 3, 2024
1 parent 24f7541 commit 4142306
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 13 deletions.
2 changes: 2 additions & 0 deletions sdk/core/core-rest-pipeline/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Bugs Fixed

- Guard against unrecognized value types in the form data policy.

### Other Changes

## 1.13.0 (2023-12-07)
Expand Down
4 changes: 4 additions & 0 deletions sdk/core/core-rest-pipeline/src/policies/formDataPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ async function prepareFormData(formData: FormDataMap, request: PipelineRequest):
}),
body: stringToUint8Array(value, "utf-8"),
});
} else if (value === undefined || value === null || typeof value !== "object") {
throw new Error(
`Unexpected value for key ${fieldName}: ${value}. Value should be serialized to string first.`,
);
} else {
// using || instead of ?? here since if value.name is empty we should create a file name
const fileName = (value as File).name || "blob";
Expand Down
8 changes: 2 additions & 6 deletions sdk/openai/openai/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
# Release History

## 1.0.0-beta.10 (Unreleased)

### Features Added

### Breaking Changes
## 1.0.0-beta.10 (2024-01-03)

### Bugs Fixed

### Other Changes
- Fix `responseFormat` behavior in `getAudioTranscription` and `getAudioTranslation` methods where request wasn't properly formed if it wasn't specified.

## 1.0.0-beta.9 (2024-01-02)

Expand Down
2 changes: 1 addition & 1 deletion sdk/openai/openai/assets.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"AssetsRepo": "Azure/azure-sdk-assets",
"AssetsRepoPrefixPath": "js",
"TagPrefix": "js/openai/openai",
"Tag": "js/openai/openai_8cdce605bc"
"Tag": "js/openai/openai_94d151367d"
}
Original file line number Diff line number Diff line change
Expand Up @@ -244,9 +244,9 @@ export async function getAudioTranslation<Format extends AudioResultFormat>(
}),
contentType: "multipart/form-data",
body: {
file: createFile(fileContent, "placeholder.wav"),
response_format,
...snakeCaseKeys(rest),
file: createFile(fileContent, "placeholder.wav"),
...(response_format ? { response_format } : {}),
},
});
if (status !== "200") {
Expand Down Expand Up @@ -310,9 +310,9 @@ export async function getAudioTranscription<Format extends AudioResultFormat>(
}),
contentType: "multipart/form-data",
body: {
file: createFile(fileContent, "placeholder.wav"),
response_format,
...snakeCaseKeys(rest),
file: createFile(fileContent, "placeholder.wav"),
...(response_format ? { response_format } : {}),
},
});
if (status !== "200") {
Expand Down
4 changes: 2 additions & 2 deletions sdk/openai/openai/src/api/client/openAIClient/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ export async function getAudioTranslation<Format extends AudioResultFormat>(
body: {
...snakeCaseKeys(rest),
file: createFile(fileContent, "placeholder.wav"),
response_format,
...(response_format ? { response_format } : {}),
},
});
if (status !== "200") {
Expand Down Expand Up @@ -684,7 +684,7 @@ export async function getAudioTranscription<Format extends AudioResultFormat>(
body: {
...snakeCaseKeys(rest),
file: createFile(fileContent, "placeholder.wav"),
response_format,
...(response_format ? { response_format } : {}),
},
});
if (status !== "200") {
Expand Down
16 changes: 16 additions & 0 deletions sdk/openai/openai/test/public/node/whisper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,22 @@ describe("OpenAI", function () {
}
});

describe("getAudioTranscription", function () {
it(`returns json transcription if responseFormat wasn't specified`, async function () {
const file = await fs.readFile(`./assets/audio/countdown.mp3`);
const res = await client.getAudioTranscription(getModel(authMethod), file);
assertAudioResult("json", res);
});
});

describe("getAudioTranslation", function () {
it(`returns json translation if responseFormat wasn't specified`, async function () {
const file = await fs.readFile(`./assets/audio/countdown.mp3`);
const res = await client.getAudioTranslation(getModel(authMethod), file);
assertAudioResult("json", res);
});
});

matrix(
[
["json", "verbose_json", "srt", "vtt", "text"],
Expand Down

0 comments on commit 4142306

Please sign in to comment.