-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: correct Requesty model fetching URL construction #7379
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,184 @@ | ||
| import { describe, it, expect, vi, beforeEach } from "vitest" | ||
| import axios from "axios" | ||
| import { getRequestyModels } from "../requesty" | ||
|
|
||
| vi.mock("axios") | ||
|
|
||
| describe("getRequestyModels", () => { | ||
| const mockAxios = axios as any | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks() | ||
| }) | ||
|
|
||
| const mockModelsResponse = { | ||
| data: { | ||
| data: [ | ||
| { | ||
| id: "test-model", | ||
| max_output_tokens: 4096, | ||
| context_window: 128000, | ||
| supports_caching: true, | ||
| supports_vision: true, | ||
| supports_computer_use: false, | ||
| supports_reasoning: false, | ||
| input_price: 3, | ||
| output_price: 15, | ||
| caching_price: 3.75, | ||
| cached_price: 0.3, | ||
| description: "Test model", | ||
| }, | ||
| ], | ||
| }, | ||
| } | ||
|
|
||
| describe("URL construction", () => { | ||
| it("should correctly append /models to default base URL", async () => { | ||
| mockAxios.get = vi.fn().mockResolvedValue(mockModelsResponse) | ||
|
|
||
| await getRequestyModels() | ||
|
|
||
| expect(mockAxios.get).toHaveBeenCalledWith("https://router.requesty.ai/v1/models", { headers: {} }) | ||
| }) | ||
|
|
||
| it("should correctly append /models to custom base URL with /v1", async () => { | ||
| mockAxios.get = vi.fn().mockResolvedValue(mockModelsResponse) | ||
|
|
||
| await getRequestyModels("https://custom.requesty.ai/v1") | ||
|
|
||
| expect(mockAxios.get).toHaveBeenCalledWith("https://custom.requesty.ai/v1/models", { headers: {} }) | ||
| }) | ||
|
|
||
| it("should correctly append /models to custom base URL with trailing slash", async () => { | ||
| mockAxios.get = vi.fn().mockResolvedValue(mockModelsResponse) | ||
|
|
||
| await getRequestyModels("https://custom.requesty.ai/v1/") | ||
|
|
||
| expect(mockAxios.get).toHaveBeenCalledWith("https://custom.requesty.ai/v1/models", { headers: {} }) | ||
| }) | ||
|
|
||
| it("should correctly append /models to custom base URL without /v1", async () => { | ||
| mockAxios.get = vi.fn().mockResolvedValue(mockModelsResponse) | ||
|
|
||
| await getRequestyModels("https://custom.requesty.ai") | ||
|
|
||
| expect(mockAxios.get).toHaveBeenCalledWith("https://custom.requesty.ai/models", { headers: {} }) | ||
| }) | ||
|
|
||
| it("should include authorization header when API key is provided", async () => { | ||
| mockAxios.get = vi.fn().mockResolvedValue(mockModelsResponse) | ||
|
|
||
| await getRequestyModels("https://custom.requesty.ai/v1", "test-api-key") | ||
|
|
||
| expect(mockAxios.get).toHaveBeenCalledWith("https://custom.requesty.ai/v1/models", { | ||
| headers: { Authorization: "Bearer test-api-key" }, | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| describe("model parsing", () => { | ||
| it("should correctly parse model information", async () => { | ||
| mockAxios.get = vi.fn().mockResolvedValue(mockModelsResponse) | ||
|
|
||
| const models = await getRequestyModels() | ||
|
|
||
| expect(models["test-model"]).toEqual({ | ||
| maxTokens: 4096, | ||
| contextWindow: 128000, | ||
| supportsPromptCache: true, | ||
| supportsImages: true, | ||
| supportsComputerUse: false, | ||
| supportsReasoningBudget: false, | ||
| supportsReasoningEffort: false, | ||
| inputPrice: 3000000, // parseApiPrice multiplies by 1,000,000 | ||
| outputPrice: 15000000, // parseApiPrice multiplies by 1,000,000 | ||
| description: "Test model", | ||
| cacheWritesPrice: 3750000, // parseApiPrice multiplies by 1,000,000 | ||
| cacheReadsPrice: 300000, // parseApiPrice multiplies by 1,000,000 | ||
| }) | ||
| }) | ||
|
|
||
| it("should handle reasoning support for Claude models", async () => { | ||
| const claudeResponse = { | ||
| data: { | ||
| data: [ | ||
| { | ||
| id: "claude-3-opus", | ||
| max_output_tokens: 4096, | ||
| context_window: 200000, | ||
| supports_caching: true, | ||
| supports_vision: true, | ||
| supports_computer_use: true, | ||
| supports_reasoning: true, | ||
| input_price: 15, | ||
| output_price: 75, | ||
| caching_price: 18.75, | ||
| cached_price: 1.5, | ||
| description: "Claude 3 Opus", | ||
| }, | ||
| ], | ||
| }, | ||
| } | ||
|
|
||
| mockAxios.get = vi.fn().mockResolvedValue(claudeResponse) | ||
|
|
||
| const models = await getRequestyModels() | ||
|
|
||
| expect(models["claude-3-opus"]).toBeDefined() | ||
| expect(models["claude-3-opus"].supportsReasoningBudget).toBe(true) | ||
| expect(models["claude-3-opus"].supportsReasoningEffort).toBe(false) | ||
| }) | ||
|
|
||
| it("should handle reasoning support for OpenAI models", async () => { | ||
| const openaiResponse = { | ||
| data: { | ||
| data: [ | ||
| { | ||
| id: "openai/gpt-4", | ||
| max_output_tokens: 4096, | ||
| context_window: 128000, | ||
| supports_caching: false, | ||
| supports_vision: true, | ||
| supports_computer_use: false, | ||
| supports_reasoning: true, | ||
| input_price: 10, | ||
| output_price: 30, | ||
| caching_price: 0, | ||
| cached_price: 0, | ||
| description: "GPT-4", | ||
| }, | ||
| ], | ||
| }, | ||
| } | ||
|
|
||
| mockAxios.get = vi.fn().mockResolvedValue(openaiResponse) | ||
|
|
||
| const models = await getRequestyModels() | ||
|
|
||
| expect(models["openai/gpt-4"]).toBeDefined() | ||
| expect(models["openai/gpt-4"].supportsReasoningBudget).toBe(false) | ||
| expect(models["openai/gpt-4"].supportsReasoningEffort).toBe(true) | ||
| }) | ||
| }) | ||
|
|
||
| describe("error handling", () => { | ||
| it("should return empty object on API error", async () => { | ||
| mockAxios.get = vi.fn().mockRejectedValue(new Error("API Error")) | ||
|
|
||
| const models = await getRequestyModels() | ||
|
|
||
| expect(models).toEqual({}) | ||
| }) | ||
|
|
||
| it("should log error details", async () => { | ||
| const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) | ||
| mockAxios.get = vi.fn().mockRejectedValue(new Error("API Error")) | ||
|
|
||
| await getRequestyModels() | ||
|
|
||
| expect(consoleErrorSpy).toHaveBeenCalledWith(expect.stringContaining("Error fetching Requesty models:")) | ||
|
|
||
| consoleErrorSpy.mockRestore() | ||
| }) | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,7 +16,11 @@ export async function getRequestyModels(baseUrl?: string, apiKey?: string): Prom | |||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const resolvedBaseUrl = toRequestyServiceUrl(baseUrl) | ||||||||||||
| const modelsUrl = new URL("models", resolvedBaseUrl) | ||||||||||||
| // Ensure the base URL ends with a slash so "models" is appended correctly | ||||||||||||
| // Without this, new URL("models", "https://custom.requesty.ai/v1") would incorrectly | ||||||||||||
| // resolve to "https://custom.requesty.ai/models" instead of "https://custom.requesty.ai/v1/models" | ||||||||||||
| const baseWithSlash = resolvedBaseUrl.endsWith("/") ? resolvedBaseUrl : `${resolvedBaseUrl}/` | ||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix looks correct and the comment is helpful! Though I'm wondering - what happens if someone provides a base URL with query parameters like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider extracting this URL construction logic into a small helper function for better testability and reusability. Something like:
Suggested change
|
||||||||||||
| const modelsUrl = new URL("models", baseWithSlash) | ||||||||||||
|
|
||||||||||||
| const response = await axios.get(modelsUrl.toString(), { headers }) | ||||||||||||
| const rawModels = response.data.data | ||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great test coverage! Consider adding a few edge cases:
?key=value)#section):8080)These would help ensure the URL construction is robust across all scenarios.