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

fix(js): Log instead of throw errors on 404 #1032

Merged
merged 3 commits into from
Sep 24, 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
2 changes: 1 addition & 1 deletion js/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@
immediatelyTriggerBatch ||
this.autoBatchQueue.size > this.pendingAutoBatchedRunLimit
) {
await this.drainAutoBatchQueue();
await this.drainAutoBatchQueue().catch(console.error);
}
if (this.autoBatchQueue.size > 0) {
this.autoBatchTimeout = setTimeout(
Expand Down Expand Up @@ -1291,7 +1291,7 @@
treeFilter?: string;
isRoot?: boolean;
dataSourceType?: string;
}): Promise<any> {

Check warning on line 1294 in js/src/client.ts

View workflow job for this annotation

GitHub Actions / Check linting

Unexpected any. Specify a different type
let projectIds_ = projectIds || [];
if (projectNames) {
projectIds_ = [
Expand Down Expand Up @@ -1579,7 +1579,7 @@
`Failed to list shared examples: ${response.status} ${response.statusText}`
);
}
return result.map((example: any) => ({

Check warning on line 1582 in js/src/client.ts

View workflow job for this annotation

GitHub Actions / Check linting

Unexpected any. Specify a different type
...example,
_hostUrl: this.getHostUrl(),
}));
Expand Down Expand Up @@ -2722,7 +2722,7 @@
}

const feedbackResult = await evaluator.evaluateRun(run_, referenceExample);
const [_, feedbacks] = await this._logEvaluationFeedback(

Check warning on line 2725 in js/src/client.ts

View workflow job for this annotation

GitHub Actions / Check linting

'_' is assigned a value but never used
feedbackResult,
run_,
sourceInfo
Expand Down Expand Up @@ -3056,7 +3056,7 @@
async _logEvaluationFeedback(
evaluatorResponse: EvaluationResult | EvaluationResults,
run?: Run,
sourceInfo?: { [key: string]: any }

Check warning on line 3059 in js/src/client.ts

View workflow job for this annotation

GitHub Actions / Check linting

Unexpected any. Specify a different type
): Promise<[results: EvaluationResult[], feedbacks: Feedback[]]> {
const evalResults: Array<EvaluationResult> =
this._selectEvalResults(evaluatorResponse);
Expand Down Expand Up @@ -3095,7 +3095,7 @@
public async logEvaluationFeedback(
evaluatorResponse: EvaluationResult | EvaluationResults,
run?: Run,
sourceInfo?: { [key: string]: any }

Check warning on line 3098 in js/src/client.ts

View workflow job for this annotation

GitHub Actions / Check linting

Unexpected any. Specify a different type
): Promise<EvaluationResult[]> {
const [results] = await this._logEvaluationFeedback(
evaluatorResponse,
Expand Down Expand Up @@ -3363,7 +3363,7 @@
promptIdentifier: string,
like: boolean
): Promise<LikePromptResponse> {
const [owner, promptName, _] = parsePromptIdentifier(promptIdentifier);

Check warning on line 3366 in js/src/client.ts

View workflow job for this annotation

GitHub Actions / Check linting

'_' is assigned a value but never used
const response = await this.caller.call(
_getFetchImplementation(),
`${this.apiUrl}/likes/${owner}/${promptName}`,
Expand Down Expand Up @@ -3468,7 +3468,7 @@
}

public async getPrompt(promptIdentifier: string): Promise<Prompt | null> {
const [owner, promptName, _] = parsePromptIdentifier(promptIdentifier);

Check warning on line 3471 in js/src/client.ts

View workflow job for this annotation

GitHub Actions / Check linting

'_' is assigned a value but never used
const response = await this.caller.call(
_getFetchImplementation(),
`${this.apiUrl}/repos/${owner}/${promptName}`,
Expand Down Expand Up @@ -3512,7 +3512,7 @@
);
}

const [owner, promptName, _] = parsePromptIdentifier(promptIdentifier);

Check warning on line 3515 in js/src/client.ts

View workflow job for this annotation

GitHub Actions / Check linting

'_' is assigned a value but never used
if (!(await this._currentTenantIsOwner(owner))) {
throw await this._ownerConflictError("create a prompt", owner);
}
Expand Down Expand Up @@ -3545,7 +3545,7 @@

public async createCommit(
promptIdentifier: string,
object: any,

Check warning on line 3548 in js/src/client.ts

View workflow job for this annotation

GitHub Actions / Check linting

Unexpected any. Specify a different type
options?: {
parentCommitHash?: string;
}
Expand All @@ -3554,7 +3554,7 @@
throw new Error("Prompt does not exist, you must create it first.");
}

const [owner, promptName, _] = parsePromptIdentifier(promptIdentifier);

Check warning on line 3557 in js/src/client.ts

View workflow job for this annotation

GitHub Actions / Check linting

'_' is assigned a value but never used
const resolvedParentCommitHash =
options?.parentCommitHash === "latest" || !options?.parentCommitHash
? await this._getLatestCommitHash(`${owner}/${promptName}`)
Expand Down
33 changes: 33 additions & 0 deletions js/src/tests/traceable.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { jest } from "@jest/globals";
import { RunTree, RunTreeConfig } from "../run_trees.js";
import { ROOT, traceable, withRunTree } from "../traceable.js";
import { getAssumedTreeFromCalls } from "./utils/tree.js";
import { mockClient } from "./utils/mock_client.js";
import { Client, overrideFetchImplementation } from "../index.js";

test("basic traceable implementation", async () => {
const { client, callSpy } = mockClient();
Expand All @@ -26,6 +28,37 @@ test("basic traceable implementation", async () => {
});
});

test("404s should only log, not throw an error", async () => {
const overriddenFetch = jest.fn(() =>
Promise.resolve({
ok: false,
status: 404,
statusText: "Expected test error",
json: () => Promise.resolve({}),
text: () => Promise.resolve("Expected test error."),
})
);
overrideFetchImplementation(overriddenFetch);
const client = new Client({
apiUrl: "https://foobar.notreal",
});
const llm = traceable(
async function* llm(input: string) {
const response = input.repeat(2).split("");
for (const char of response) {
yield char;
}
},
{ client, tracingEnabled: true }
);

// eslint-disable-next-line @typescript-eslint/no-unused-vars
for await (const _ of llm("Hello world")) {
// pass
}
expect(overriddenFetch).toHaveBeenCalled();
});

test("nested traceable implementation", async () => {
const { client, callSpy } = mockClient();

Expand Down
Loading