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(client): add ._request_id property to object responses #1078

Merged
merged 3 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,17 @@ Error codes are as followed:
| >=500 | `InternalServerError` |
| N/A | `APIConnectionError` |

## Request IDs

> For more information on debugging requests, see [these docs](https://platform.openai.com/docs/api-reference/debugging-requests)

All object responses in the SDK provide a `_request_id` property which is added from the `x-request-id` response header so that you can quickly log failing requests and report them back to OpenAI.

```ts
const completion = await client.chat.completions.create({ messages: [{ role: 'user', content: 'Say this is a test' }], model: 'gpt-4' });
console.log(completion._request_id) // req_123
```

## Microsoft Azure OpenAI

To use this library with [Azure OpenAI](https://learn.microsoft.com/azure/ai-services/openai/overview), use the `AzureOpenAI`
Expand Down
58 changes: 42 additions & 16 deletions src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type APIResponseProps = {
controller: AbortController;
};

async function defaultParseResponse<T>(props: APIResponseProps): Promise<T> {
async function defaultParseResponse<T>(props: APIResponseProps): Promise<WithRequestID<T>> {
const { response } = props;
if (props.options.stream) {
debug('response', response.status, response.url, response.headers, response.body);
Expand All @@ -54,11 +54,11 @@ async function defaultParseResponse<T>(props: APIResponseProps): Promise<T> {

// fetch refuses to read the body when the status code is 204.
if (response.status === 204) {
return null as T;
return null as WithRequestID<T>;
}

if (props.options.__binaryResponse) {
return response as unknown as T;
return response as unknown as WithRequestID<T>;
}

const contentType = response.headers.get('content-type');
Expand All @@ -69,26 +69,44 @@ async function defaultParseResponse<T>(props: APIResponseProps): Promise<T> {

debug('response', response.status, response.url, response.headers, json);

return json as T;
return _addRequestID(json, response);
}

const text = await response.text();
debug('response', response.status, response.url, response.headers, text);

// TODO handle blob, arraybuffer, other content types, etc.
return text as unknown as T;
return text as unknown as WithRequestID<T>;
}

type WithRequestID<T> =
T extends Array<any> | Response | AbstractPage<any> ? T
: T extends Record<string, any> ? T & { _request_id?: string | null }
: T;

function _addRequestID<T>(value: T, response: Response): WithRequestID<T> {
if (!value || typeof value !== 'object' || Array.isArray(value)) {
return value as WithRequestID<T>;
}

return Object.defineProperty(value, '_request_id', {
value: response.headers.get('x-request-id'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this error if the value doesn't exist, or return undefined?

Copy link
Collaborator Author

@RobertCraigie RobertCraigie Sep 17, 2024

Choose a reason for hiding this comment

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

it'll return undefined or null

enumerable: false,
}) as WithRequestID<T>;
}

/**
* A subclass of `Promise` providing additional helper methods
* for interacting with the SDK.
*/
export class APIPromise<T> extends Promise<T> {
private parsedPromise: Promise<T> | undefined;
export class APIPromise<T> extends Promise<WithRequestID<T>> {
private parsedPromise: Promise<WithRequestID<T>> | undefined;

constructor(
private responsePromise: Promise<APIResponseProps>,
private parseResponse: (props: APIResponseProps) => PromiseOrValue<T> = defaultParseResponse,
private parseResponse: (
props: APIResponseProps,
) => PromiseOrValue<WithRequestID<T>> = defaultParseResponse,
) {
super((resolve) => {
// this is maybe a bit weird but this has to be a no-op to not implicitly
Expand All @@ -99,7 +117,9 @@ export class APIPromise<T> extends Promise<T> {
}

_thenUnwrap<U>(transform: (data: T) => U): APIPromise<U> {
return new APIPromise(this.responsePromise, async (props) => transform(await this.parseResponse(props)));
return new APIPromise(this.responsePromise, async (props) =>
_addRequestID(transform(await this.parseResponse(props)), props.response),
);
}

/**
Expand Down Expand Up @@ -136,27 +156,27 @@ export class APIPromise<T> extends Promise<T> {
return { data, response };
}

private parse(): Promise<T> {
private parse(): Promise<WithRequestID<T>> {
if (!this.parsedPromise) {
this.parsedPromise = this.responsePromise.then(this.parseResponse);
this.parsedPromise = this.responsePromise.then(this.parseResponse) as any as Promise<WithRequestID<T>>;
}
return this.parsedPromise;
}

override then<TResult1 = T, TResult2 = never>(
onfulfilled?: ((value: T) => TResult1 | PromiseLike<TResult1>) | undefined | null,
override then<TResult1 = WithRequestID<T>, TResult2 = never>(
onfulfilled?: ((value: WithRequestID<T>) => TResult1 | PromiseLike<TResult1>) | undefined | null,
onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | undefined | null,
): Promise<TResult1 | TResult2> {
return this.parse().then(onfulfilled, onrejected);
}

override catch<TResult = never>(
onrejected?: ((reason: any) => TResult | PromiseLike<TResult>) | undefined | null,
): Promise<T | TResult> {
): Promise<WithRequestID<T> | TResult> {
return this.parse().catch(onrejected);
}

override finally(onfinally?: (() => void) | undefined | null): Promise<T> {
override finally(onfinally?: (() => void) | undefined | null): Promise<WithRequestID<T>> {
return this.parse().finally(onfinally);
}
}
Expand Down Expand Up @@ -706,7 +726,13 @@ export class PagePromise<
) {
super(
request,
async (props) => new Page(client, props.response, await defaultParseResponse(props), props.options),
async (props) =>
new Page(
client,
props.response,
await defaultParseResponse(props),
props.options,
) as WithRequestID<PageClass>,
);
}

Expand Down
104 changes: 103 additions & 1 deletion tests/responses.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { createResponseHeaders } from 'openai/core';
import { APIPromise, createResponseHeaders } from 'openai/core';
import OpenAI from 'openai/index';
import { Headers } from 'openai/_shims/index';
import { Response } from 'node-fetch';
import { compareType } from './utils/typing';

describe('response parsing', () => {
// TODO: test unicode characters
Expand All @@ -23,3 +26,102 @@ describe('response parsing', () => {
expect(headers['content-type']).toBe('text/xml, application/json');
});
});

describe('request id', () => {
test('types', () => {
compareType<Awaited<APIPromise<string>>, string>(true);
compareType<Awaited<APIPromise<number>>, number>(true);
compareType<Awaited<APIPromise<null>>, null>(true);
compareType<Awaited<APIPromise<void>>, void>(true);
compareType<Awaited<APIPromise<Response>>, Response>(true);
compareType<Awaited<APIPromise<Response>>, Response>(true);
compareType<Awaited<APIPromise<{ foo: string }>>, { foo: string } & { _request_id?: string | null }>(
true,
);
compareType<Awaited<APIPromise<Array<{ foo: string }>>>, Array<{ foo: string }>>(true);
});

test('object response', async () => {
const client = new OpenAI({
apiKey: 'dummy',
fetch: async () =>
new Response(JSON.stringify({ id: 'bar' }), {
headers: { 'x-request-id': 'req_id_xxx', 'content-type': 'application/json' },
}),
});

const rsp = await client.chat.completions.create({ messages: [], model: 'gpt-4' });
expect(rsp.id).toBe('bar');
expect(rsp._request_id).toBe('req_id_xxx');
expect(JSON.stringify(rsp)).toBe('{"id":"bar"}');
});

test('envelope response', async () => {
const promise = new APIPromise<{ data: { foo: string } }>(
(async () => {
return {
response: new Response(JSON.stringify({ data: { foo: 'bar' } }), {
headers: { 'x-request-id': 'req_id_xxx', 'content-type': 'application/json' },
}),
controller: {} as any,
options: {} as any,
};
})(),
)._thenUnwrap((d) => d.data);

const rsp = await promise;
expect(rsp.foo).toBe('bar');
expect(rsp._request_id).toBe('req_id_xxx');
});

test('page response', async () => {
const client = new OpenAI({
apiKey: 'dummy',
fetch: async () =>
new Response(JSON.stringify({ data: [{ foo: 'bar' }] }), {
headers: { 'x-request-id': 'req_id_xxx', 'content-type': 'application/json' },
}),
});

const page = await client.fineTuning.jobs.list();
expect(page.data).toMatchObject([{ foo: 'bar' }]);
expect((page as any)._request_id).toBeUndefined();
});

test('array response', async () => {
const promise = new APIPromise<Array<{ foo: string }>>(
(async () => {
return {
response: new Response(JSON.stringify([{ foo: 'bar' }]), {
headers: { 'x-request-id': 'req_id_xxx', 'content-type': 'application/json' },
}),
controller: {} as any,
options: {} as any,
};
})(),
);

const rsp = await promise;
expect(rsp.length).toBe(1);
expect(rsp[0]).toMatchObject({ foo: 'bar' });
expect((rsp as any)._request_id).toBeUndefined();
});

test('string response', async () => {
const promise = new APIPromise<string>(
(async () => {
return {
response: new Response('hello world', {
headers: { 'x-request-id': 'req_id_xxx', 'content-type': 'application/text' },
}),
controller: {} as any,
options: {} as any,
};
})(),
);

const result = await promise;
expect(result).toBe('hello world');
expect((result as any)._request_id).toBeUndefined();
});
});
9 changes: 9 additions & 0 deletions tests/utils/typing.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
type Equal<X, Y> = (<T>() => T extends X ? 1 : 2) extends <T>() => T extends Y ? 1 : 2 ? true : false;

export const expectType = <T>(_expression: T): void => {
return;
};

export const compareType = <T1, T2>(_expression: Equal<T1, T2>): void => {
return;
};