mirror of
https://github.com/pacnpal/Roo-Code.git
synced 2025-12-20 04:11:10 -05:00
Merge pull request #317 from RooVetGit/better_retry_ux
Improve the UX for API request retries
This commit is contained in:
@@ -833,15 +833,15 @@ export class Cline {
|
|||||||
} catch (error) {
|
} catch (error) {
|
||||||
// note that this api_req_failed ask is unique in that we only present this option if the api hasn't streamed any content yet (ie it fails on the first chunk due), as it would allow them to hit a retry button. However if the api failed mid-stream, it could be in any arbitrary state where some tools may have executed, so that error is handled differently and requires cancelling the task entirely.
|
// note that this api_req_failed ask is unique in that we only present this option if the api hasn't streamed any content yet (ie it fails on the first chunk due), as it would allow them to hit a retry button. However if the api failed mid-stream, it could be in any arbitrary state where some tools may have executed, so that error is handled differently and requires cancelling the task entirely.
|
||||||
if (alwaysApproveResubmit) {
|
if (alwaysApproveResubmit) {
|
||||||
|
const errorMsg = error.message ?? "Unknown error"
|
||||||
const requestDelay = requestDelaySeconds || 5
|
const requestDelay = requestDelaySeconds || 5
|
||||||
// Automatically retry with delay
|
// Automatically retry with delay
|
||||||
await this.say(
|
// Show countdown timer in error color
|
||||||
"error",
|
for (let i = requestDelay; i > 0; i--) {
|
||||||
`${error.message ?? "Unknown error"} ↺ Retrying in ${requestDelay} seconds...`,
|
await this.say("api_req_retry_delayed", `${errorMsg}\n\nRetrying in ${i} seconds...`, undefined, true)
|
||||||
)
|
await delay(1000)
|
||||||
await this.say("api_req_retry_delayed")
|
}
|
||||||
await delay(requestDelay * 1000)
|
await this.say("api_req_retry_delayed", `${errorMsg}\n\nRetrying now...`, undefined, false)
|
||||||
await this.say("api_req_retried")
|
|
||||||
// delegate generator output from the recursive call
|
// delegate generator output from the recursive call
|
||||||
yield* this.attemptApiRequest(previousApiReqIndex)
|
yield* this.attemptApiRequest(previousApiReqIndex)
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -628,6 +628,133 @@ describe('Cline', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should handle API retry with countdown', async () => {
|
||||||
|
const cline = new Cline(
|
||||||
|
mockProvider,
|
||||||
|
mockApiConfig,
|
||||||
|
undefined,
|
||||||
|
false,
|
||||||
|
undefined,
|
||||||
|
'test task'
|
||||||
|
);
|
||||||
|
|
||||||
|
// Mock delay to track countdown timing
|
||||||
|
const mockDelay = jest.fn().mockResolvedValue(undefined);
|
||||||
|
jest.spyOn(require('delay'), 'default').mockImplementation(mockDelay);
|
||||||
|
|
||||||
|
// Mock say to track messages
|
||||||
|
const saySpy = jest.spyOn(cline, 'say');
|
||||||
|
|
||||||
|
// Create a stream that fails on first chunk
|
||||||
|
const mockError = new Error('API Error');
|
||||||
|
const mockFailedStream = {
|
||||||
|
async *[Symbol.asyncIterator]() {
|
||||||
|
throw mockError;
|
||||||
|
},
|
||||||
|
async next() {
|
||||||
|
throw mockError;
|
||||||
|
},
|
||||||
|
async return() {
|
||||||
|
return { done: true, value: undefined };
|
||||||
|
},
|
||||||
|
async throw(e: any) {
|
||||||
|
throw e;
|
||||||
|
},
|
||||||
|
async [Symbol.asyncDispose]() {
|
||||||
|
// Cleanup
|
||||||
|
}
|
||||||
|
} as AsyncGenerator<ApiStreamChunk>;
|
||||||
|
|
||||||
|
// Create a successful stream for retry
|
||||||
|
const mockSuccessStream = {
|
||||||
|
async *[Symbol.asyncIterator]() {
|
||||||
|
yield { type: 'text', text: 'Success' };
|
||||||
|
},
|
||||||
|
async next() {
|
||||||
|
return { done: true, value: { type: 'text', text: 'Success' } };
|
||||||
|
},
|
||||||
|
async return() {
|
||||||
|
return { done: true, value: undefined };
|
||||||
|
},
|
||||||
|
async throw(e: any) {
|
||||||
|
throw e;
|
||||||
|
},
|
||||||
|
async [Symbol.asyncDispose]() {
|
||||||
|
// Cleanup
|
||||||
|
}
|
||||||
|
} as AsyncGenerator<ApiStreamChunk>;
|
||||||
|
|
||||||
|
// Mock createMessage to fail first then succeed
|
||||||
|
let firstAttempt = true;
|
||||||
|
jest.spyOn(cline.api, 'createMessage').mockImplementation(() => {
|
||||||
|
if (firstAttempt) {
|
||||||
|
firstAttempt = false;
|
||||||
|
return mockFailedStream;
|
||||||
|
}
|
||||||
|
return mockSuccessStream;
|
||||||
|
});
|
||||||
|
|
||||||
|
// Set alwaysApproveResubmit and requestDelaySeconds
|
||||||
|
mockProvider.getState = jest.fn().mockResolvedValue({
|
||||||
|
alwaysApproveResubmit: true,
|
||||||
|
requestDelaySeconds: 3
|
||||||
|
});
|
||||||
|
|
||||||
|
// Mock previous API request message
|
||||||
|
cline.clineMessages = [{
|
||||||
|
ts: Date.now(),
|
||||||
|
type: 'say',
|
||||||
|
say: 'api_req_started',
|
||||||
|
text: JSON.stringify({
|
||||||
|
tokensIn: 100,
|
||||||
|
tokensOut: 50,
|
||||||
|
cacheWrites: 0,
|
||||||
|
cacheReads: 0,
|
||||||
|
request: 'test request'
|
||||||
|
})
|
||||||
|
}];
|
||||||
|
|
||||||
|
// Trigger API request
|
||||||
|
const iterator = cline.attemptApiRequest(0);
|
||||||
|
await iterator.next();
|
||||||
|
|
||||||
|
// Verify countdown messages
|
||||||
|
expect(saySpy).toHaveBeenCalledWith(
|
||||||
|
'api_req_retry_delayed',
|
||||||
|
expect.stringContaining('Retrying in 3 seconds'),
|
||||||
|
undefined,
|
||||||
|
true
|
||||||
|
);
|
||||||
|
expect(saySpy).toHaveBeenCalledWith(
|
||||||
|
'api_req_retry_delayed',
|
||||||
|
expect.stringContaining('Retrying in 2 seconds'),
|
||||||
|
undefined,
|
||||||
|
true
|
||||||
|
);
|
||||||
|
expect(saySpy).toHaveBeenCalledWith(
|
||||||
|
'api_req_retry_delayed',
|
||||||
|
expect.stringContaining('Retrying in 1 seconds'),
|
||||||
|
undefined,
|
||||||
|
true
|
||||||
|
);
|
||||||
|
expect(saySpy).toHaveBeenCalledWith(
|
||||||
|
'api_req_retry_delayed',
|
||||||
|
expect.stringContaining('Retrying now'),
|
||||||
|
undefined,
|
||||||
|
false
|
||||||
|
);
|
||||||
|
|
||||||
|
// Verify delay was called correctly
|
||||||
|
expect(mockDelay).toHaveBeenCalledTimes(3);
|
||||||
|
expect(mockDelay).toHaveBeenCalledWith(1000);
|
||||||
|
|
||||||
|
// Verify error message content
|
||||||
|
const errorMessage = saySpy.mock.calls.find(
|
||||||
|
call => call[1]?.includes(mockError.message)
|
||||||
|
)?.[1];
|
||||||
|
expect(errorMessage).toBe(`${mockError.message}\n\nRetrying in 3 seconds...`);
|
||||||
|
});
|
||||||
|
|
||||||
describe('loadContext', () => {
|
describe('loadContext', () => {
|
||||||
it('should process mentions in task and feedback tags', async () => {
|
it('should process mentions in task and feedback tags', async () => {
|
||||||
const cline = new Cline(
|
const cline = new Cline(
|
||||||
|
|||||||
@@ -154,6 +154,8 @@ export const ChatRowContent = ({
|
|||||||
style={{ color: successColor, marginBottom: "-1.5px" }}></span>,
|
style={{ color: successColor, marginBottom: "-1.5px" }}></span>,
|
||||||
<span style={{ color: successColor, fontWeight: "bold" }}>Task Completed</span>,
|
<span style={{ color: successColor, fontWeight: "bold" }}>Task Completed</span>,
|
||||||
]
|
]
|
||||||
|
case "api_req_retry_delayed":
|
||||||
|
return []
|
||||||
case "api_req_started":
|
case "api_req_started":
|
||||||
const getIconSpan = (iconName: string, color: string) => (
|
const getIconSpan = (iconName: string, color: string) => (
|
||||||
<div
|
<div
|
||||||
@@ -211,15 +213,7 @@ export const ChatRowContent = ({
|
|||||||
default:
|
default:
|
||||||
return [null, null]
|
return [null, null]
|
||||||
}
|
}
|
||||||
}, [
|
}, [type, isCommandExecuting, message, isMcpServerResponding, apiReqCancelReason, cost, apiRequestFailedMessage])
|
||||||
type,
|
|
||||||
cost,
|
|
||||||
apiRequestFailedMessage,
|
|
||||||
isCommandExecuting,
|
|
||||||
apiReqCancelReason,
|
|
||||||
isMcpServerResponding,
|
|
||||||
message.text,
|
|
||||||
])
|
|
||||||
|
|
||||||
const headerStyle: React.CSSProperties = {
|
const headerStyle: React.CSSProperties = {
|
||||||
display: "flex",
|
display: "flex",
|
||||||
|
|||||||
@@ -192,6 +192,9 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie
|
|||||||
case "say":
|
case "say":
|
||||||
// don't want to reset since there could be a "say" after an "ask" while ask is waiting for response
|
// don't want to reset since there could be a "say" after an "ask" while ask is waiting for response
|
||||||
switch (lastMessage.say) {
|
switch (lastMessage.say) {
|
||||||
|
case "api_req_retry_delayed":
|
||||||
|
setTextAreaDisabled(true)
|
||||||
|
break
|
||||||
case "api_req_started":
|
case "api_req_started":
|
||||||
if (secondLastMessage?.ask === "command_output") {
|
if (secondLastMessage?.ask === "command_output") {
|
||||||
// if the last ask is a command_output, and we receive an api_req_started, then that means the command has finished and we don't need input from the user anymore (in every other case, the user has to interact with input field or buttons to continue, which does the following automatically)
|
// if the last ask is a command_output, and we receive an api_req_started, then that means the command has finished and we don't need input from the user anymore (in every other case, the user has to interact with input field or buttons to continue, which does the following automatically)
|
||||||
@@ -466,6 +469,9 @@ const ChatView = ({ isHidden, showAnnouncement, hideAnnouncement, showHistoryVie
|
|||||||
case "api_req_finished": // combineApiRequests removes this from modifiedMessages anyways
|
case "api_req_finished": // combineApiRequests removes this from modifiedMessages anyways
|
||||||
case "api_req_retried": // this message is used to update the latest api_req_started that the request was retried
|
case "api_req_retried": // this message is used to update the latest api_req_started that the request was retried
|
||||||
return false
|
return false
|
||||||
|
case "api_req_retry_delayed":
|
||||||
|
// Only show the retry message if it's the last message
|
||||||
|
return message === modifiedMessages.at(-1)
|
||||||
case "text":
|
case "text":
|
||||||
// Sometimes cline returns an empty text message, we don't want to render these. (We also use a say text for user messages, so in case they just sent images we still render that)
|
// Sometimes cline returns an empty text message, we don't want to render these. (We also use a say text for user messages, so in case they just sent images we still render that)
|
||||||
if ((message.text ?? "") === "" && (message.images?.length ?? 0) === 0) {
|
if ((message.text ?? "") === "" && (message.images?.length ?? 0) === 0) {
|
||||||
|
|||||||
Reference in New Issue
Block a user