Conversation
|
E2E Tests 🚀 |
| const mockConfig = { | ||
| get: (key: string, defaultValue?: any) => { | ||
| const configValues: Record<string, any> = { | ||
| 'authHost': 'https://auth.test.posit.cloud', |
There was a problem hiding this comment.
should we use less realistic urls in this file, like auth.example.com and similar?
| suite('Rate limit error handling (Vercel SDK path)', () => { | ||
| test('throws error with retry-after when rate limited with header', () => { | ||
| const rateLimitError = createVercelRateLimitError('30', 'https://api.posit.cloud/anthropic/v1/messages'); | ||
|
|
||
| assert.throws( | ||
| () => (model as any).handleStreamError(rateLimitError), | ||
| (error: Error) => { | ||
| assertRateLimitErrorWithRetry(error, '30'); | ||
| return true; | ||
| } | ||
| ); | ||
| }); |
There was a problem hiding this comment.
I think this test and a few others in here are already covered in the other anthropic tests. What do you think about removing them from here?
There was a problem hiding this comment.
I think it's good to keep them to ensure the Posit provider is handling the retry-after in case any code in there changes. It does add several seconds to testing but it might catch something.
| @@ -0,0 +1,92 @@ | |||
| /*--------------------------------------------------------------------------------------------- | |||
| * Copyright (C) 2025-2026 Posit Software, PBC. All rights reserved. | |||
There was a problem hiding this comment.
| * Copyright (C) 2025-2026 Posit Software, PBC. All rights reserved. | |
| * Copyright (C) 2026 Posit Software, PBC. All rights reserved. |
| @@ -0,0 +1,267 @@ | |||
| /*--------------------------------------------------------------------------------------------- | |||
| * Copyright (C) 2025-2026 Posit Software, PBC. All rights reserved. | |||
There was a problem hiding this comment.
| * Copyright (C) 2025-2026 Posit Software, PBC. All rights reserved. | |
| * Copyright (C) 2026 Posit Software, PBC. All rights reserved. |
Do you still have a mitm proxy tool? You could try intercepting and returning a 429 when making a request to anthropic |
Oh good idea! |
|
It was a bit tricky to understand what was happening with Proxyman but it revealed that the Vercel provider wasn't handling the rate limit error correctly. I've added a fix for it. The Vercel SDK is set to auto-retry and the failure will wrap the rate limit error in a retry error. 
|
sharon-wang
left a comment
sharon-wang
left a comment
There was a problem hiding this comment.
a couple comments, otherwise LGTM!
| * | ||
| * @param error - The error to check | ||
| * @param providerName - The name of the provider for the error message prefix | ||
| * @returns true if the error was handled (and thrown), false otherwise |
There was a problem hiding this comment.
should this and the vercel version of this still return boolean? the true case isn't returned for either. Maybe we can just not return anything?
| if (retryAfter) { | ||
| throw new Error(`[${providerName}] Rate limit exceeded. Please retry after ${retryAfter} seconds.`); | ||
| } | ||
| throw new Error(`[${providerName}] Rate limit exceeded. Please try again later.`); |
There was a problem hiding this comment.
should we add some logging here and in the vercel version below so the error is logged in output as well?
2 participants
Address #11001
Returns an error indicating that the rate limit has been reached and to retry after the time specified in the response header.
The providers have been updated to check for
retry-afterin the response header when an error is encountered.I'm not sure how to replicate this since I don't have a way to rate limit myself but the tests can verify the handling.
Release Notes
New Features
Bug Fixes
QA Notes