Skip to content

Commit 537e927

Browse files
jdneojdneo
committed
fix: enhance error handling in appendToolCallStatus method
1 parent ac5d9cc commit 537e927

2 files changed

Lines changed: 238 additions & 1 deletion

File tree

‎com.microsoft.copilot.eclipse.ui.test/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidgetToolCallStatusTest.java‎

Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
package com.microsoft.copilot.eclipse.ui.chat;
5+
6+
import static org.junit.jupiter.api.Assertions.assertNotNull;
7+
import static org.junit.jupiter.api.Assertions.assertTrue;
8+
import static org.mockito.Mockito.lenient;
9+
import static org.mockito.Mockito.mock;
10+
import static org.mockito.Mockito.mockStatic;
11+
import static org.mockito.Mockito.when;
12+
13+
import java.lang.reflect.Field;
14+
import java.util.Map;
15+
16+
import org.eclipse.swt.SWT;
17+
import org.eclipse.swt.custom.StyledText;
18+
import org.eclipse.swt.widgets.Display;
19+
import org.eclipse.swt.widgets.Shell;
20+
import org.junit.jupiter.api.AfterEach;
21+
import org.junit.jupiter.api.BeforeEach;
22+
import org.junit.jupiter.api.Test;
23+
import org.junit.jupiter.api.extension.ExtendWith;
24+
import org.mockito.Mock;
25+
import org.mockito.MockedStatic;
26+
import org.mockito.junit.jupiter.MockitoExtension;
27+
28+
import com.google.gson.Gson;
29+
import com.microsoft.copilot.eclipse.core.lsp.protocol.AgentToolCall;
30+
import com.microsoft.copilot.eclipse.ui.CopilotUi;
31+
import com.microsoft.copilot.eclipse.ui.chat.services.AvatarService;
32+
import com.microsoft.copilot.eclipse.ui.chat.services.ChatFontService;
33+
import com.microsoft.copilot.eclipse.ui.chat.services.ChatServiceManager;
34+
import com.microsoft.copilot.eclipse.ui.utils.SwtUtils;
35+
36+
/**
37+
* Verifies that {@link BaseTurnWidget#appendToolCallStatus} renders text for
38+
* tool calls that finish with status {@code error}.
39+
*/
40+
@ExtendWith(MockitoExtension.class)
41+
class BaseTurnWidgetToolCallStatusTest {
42+
43+
private static final String TURN_ID = "turn-1";
44+
private static final String TOOL_CALL_ID = "tool-call-1";
45+
private static final String ERROR_MESSAGE = "file not found";
46+
private static final String PROGRESS_MESSAGE = "Editing file.txt";
47+
48+
private Shell shell;
49+
50+
@Mock
51+
private ChatServiceManager mockChatServiceManager;
52+
@Mock
53+
private AvatarService mockAvatarService;
54+
@Mock
55+
private ChatFontService mockChatFontService;
56+
57+
@BeforeEach
58+
void setUp() {
59+
lenient().when(mockChatServiceManager.getAvatarService()).thenReturn(mockAvatarService);
60+
lenient().when(mockChatServiceManager.getChatFontService()).thenReturn(mockChatFontService);
61+
lenient().when(mockAvatarService.getAvatarForCopilot()).thenReturn(null);
62+
63+
SwtUtils.invokeOnDisplayThread(() -> shell = new Shell(Display.getDefault()));
64+
}
65+
66+
@AfterEach
67+
void tearDown() {
68+
SwtUtils.invokeOnDisplayThread(() -> {
69+
if (shell != null && !shell.isDisposed()) {
70+
shell.dispose();
71+
}
72+
});
73+
}
74+
75+
@Test
76+
void appendToolCallStatus_errorWithMessage_rendersErrorText() {
77+
SwtUtils.invokeOnDisplayThread(() -> {
78+
try (MockedStatic<CopilotUi> copilotUiMock = mockStatic(CopilotUi.class)) {
79+
CopilotUi mockPlugin = mock(CopilotUi.class);
80+
copilotUiMock.when(CopilotUi::getPlugin).thenReturn(mockPlugin);
81+
lenient().when(mockPlugin.getChatServiceManager()).thenReturn(mockChatServiceManager);
82+
83+
CopilotTurnWidget widget = new CopilotTurnWidget(shell, SWT.NONE, mockChatServiceManager, TURN_ID);
84+
85+
widget.appendToolCallStatus(buildToolCall("error", PROGRESS_MESSAGE, ERROR_MESSAGE));
86+
87+
StyledText text = getStatusLabelText(widget, TOOL_CALL_ID);
88+
assertNotNull(text, "Status label text must be created when an error event is delivered");
89+
assertTrue(text.getText().contains(ERROR_MESSAGE),
90+
"Expected error message to be rendered next to the error icon, got: '" + text.getText() + "'");
91+
}
92+
});
93+
}
94+
95+
@Test
96+
void appendToolCallStatus_errorWithoutProgressMessage_isStillProcessed() {
97+
SwtUtils.invokeOnDisplayThread(() -> {
98+
try (MockedStatic<CopilotUi> copilotUiMock = mockStatic(CopilotUi.class)) {
99+
CopilotUi mockPlugin = mock(CopilotUi.class);
100+
copilotUiMock.when(CopilotUi::getPlugin).thenReturn(mockPlugin);
101+
lenient().when(mockPlugin.getChatServiceManager()).thenReturn(mockChatServiceManager);
102+
103+
CopilotTurnWidget widget = new CopilotTurnWidget(shell, SWT.NONE, mockChatServiceManager, TURN_ID);
104+
105+
// No progressMessage (e.g., tool failed before the running event was dispatched server-side).
106+
widget.appendToolCallStatus(buildToolCall("error", null, ERROR_MESSAGE));
107+
108+
StyledText text = getStatusLabelText(widget, TOOL_CALL_ID);
109+
assertNotNull(text, "Error event without progressMessage must still be processed");
110+
assertTrue(text.getText().contains(ERROR_MESSAGE),
111+
"Expected error message to be rendered, got: '" + text.getText() + "'");
112+
}
113+
});
114+
}
115+
116+
@Test
117+
void appendToolCallStatus_runningThenError_replacesProgressTextWithErrorText() {
118+
SwtUtils.invokeOnDisplayThread(() -> {
119+
try (MockedStatic<CopilotUi> copilotUiMock = mockStatic(CopilotUi.class)) {
120+
CopilotUi mockPlugin = mock(CopilotUi.class);
121+
copilotUiMock.when(CopilotUi::getPlugin).thenReturn(mockPlugin);
122+
lenient().when(mockPlugin.getChatServiceManager()).thenReturn(mockChatServiceManager);
123+
124+
CopilotTurnWidget widget = new CopilotTurnWidget(shell, SWT.NONE, mockChatServiceManager, TURN_ID);
125+
126+
// First the running event sets the progress text.
127+
widget.appendToolCallStatus(buildToolCall("running", PROGRESS_MESSAGE, null));
128+
StyledText runningText = getStatusLabelText(widget, TOOL_CALL_ID);
129+
assertNotNull(runningText, "Running event should have populated text");
130+
assertTrue(runningText.getText().contains(PROGRESS_MESSAGE),
131+
"Running text should be visible before failure, got: '" + runningText.getText() + "'");
132+
133+
// Then the same tool call fails: the error text must overwrite the stale running text.
134+
widget.appendToolCallStatus(buildToolCall("error", PROGRESS_MESSAGE, ERROR_MESSAGE));
135+
136+
StyledText text = getStatusLabelText(widget, TOOL_CALL_ID);
137+
assertNotNull(text);
138+
assertTrue(text.getText().contains(ERROR_MESSAGE),
139+
"Error text should replace stale running text, got: '" + text.getText() + "'");
140+
}
141+
});
142+
}
143+
144+
@Test
145+
void appendToolCallStatus_errorFallsBackToProgressMessage_whenErrorIsBlank() {
146+
SwtUtils.invokeOnDisplayThread(() -> {
147+
try (MockedStatic<CopilotUi> copilotUiMock = mockStatic(CopilotUi.class)) {
148+
CopilotUi mockPlugin = mock(CopilotUi.class);
149+
copilotUiMock.when(CopilotUi::getPlugin).thenReturn(mockPlugin);
150+
lenient().when(mockPlugin.getChatServiceManager()).thenReturn(mockChatServiceManager);
151+
152+
CopilotTurnWidget widget = new CopilotTurnWidget(shell, SWT.NONE, mockChatServiceManager, TURN_ID);
153+
154+
// Whitespace-only error must fall back to progressMessage rather than rendering empty text.
155+
widget.appendToolCallStatus(buildToolCall("error", PROGRESS_MESSAGE, " "));
156+
157+
StyledText text = getStatusLabelText(widget, TOOL_CALL_ID);
158+
assertNotNull(text, "Status label text must be created on error even when only progressMessage is present");
159+
assertTrue(text.getText().contains(PROGRESS_MESSAGE),
160+
"Expected progressMessage to be used as fallback, got: '" + text.getText() + "'");
161+
}
162+
});
163+
}
164+
165+
@Test
166+
void appendToolCallStatus_errorFallsBackToProgressMessage_whenErrorIsNull() {
167+
SwtUtils.invokeOnDisplayThread(() -> {
168+
try (MockedStatic<CopilotUi> copilotUiMock = mockStatic(CopilotUi.class)) {
169+
CopilotUi mockPlugin = mock(CopilotUi.class);
170+
copilotUiMock.when(CopilotUi::getPlugin).thenReturn(mockPlugin);
171+
lenient().when(mockPlugin.getChatServiceManager()).thenReturn(mockChatServiceManager);
172+
173+
CopilotTurnWidget widget = new CopilotTurnWidget(shell, SWT.NONE, mockChatServiceManager, TURN_ID);
174+
175+
widget.appendToolCallStatus(buildToolCall("error", PROGRESS_MESSAGE, null));
176+
177+
StyledText text = getStatusLabelText(widget, TOOL_CALL_ID);
178+
assertNotNull(text);
179+
assertTrue(text.getText().contains(PROGRESS_MESSAGE),
180+
"Expected progressMessage to be used as fallback, got: '" + text.getText() + "'");
181+
}
182+
});
183+
}
184+
185+
private static AgentToolCall buildToolCall(String status, String progressMessage, String error) {
186+
Gson gson = new Gson();
187+
java.util.Map<String, Object> fields = new java.util.HashMap<>();
188+
fields.put("id", TOOL_CALL_ID);
189+
fields.put("name", "edit_file");
190+
fields.put("status", status);
191+
if (progressMessage != null) {
192+
fields.put("progressMessage", progressMessage);
193+
}
194+
if (error != null) {
195+
fields.put("error", error);
196+
}
197+
return gson.fromJson(gson.toJson(fields), AgentToolCall.class);
198+
}
199+
200+
private static StyledText getStatusLabelText(BaseTurnWidget widget, String toolCallId) {
201+
@SuppressWarnings("unchecked")
202+
Map<String, AgentStatusLabel> labels = (Map<String, AgentStatusLabel>) getField(widget, "statusLabels");
203+
AgentStatusLabel label = labels.get(toolCallId);
204+
assertNotNull(label, "AgentStatusLabel for tool call '" + toolCallId + "' should have been created");
205+
Object markupViewer = getField(label, "textLabel");
206+
if (markupViewer == null) {
207+
return null;
208+
}
209+
try {
210+
return (StyledText) markupViewer.getClass().getMethod("getTextWidget").invoke(markupViewer);
211+
} catch (ReflectiveOperationException e) {
212+
throw new RuntimeException("Failed to obtain StyledText from textLabel", e);
213+
}
214+
}
215+
216+
private static Object getField(Object target, String name) {
217+
Class<?> cls = target.getClass();
218+
while (cls != null) {
219+
try {
220+
Field f = cls.getDeclaredField(name);
221+
f.setAccessible(true);
222+
return f.get(target);
223+
} catch (NoSuchFieldException e) {
224+
cls = cls.getSuperclass();
225+
} catch (IllegalAccessException e) {
226+
throw new RuntimeException(e);
227+
}
228+
}
229+
throw new RuntimeException("Field '" + name + "' not found on " + target.getClass());
230+
}
231+
}

‎com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,8 @@ public void appendMessage(String message) {
226226
* @param toolCall the tool call of the agent turn
227227
*/
228228
public void appendToolCallStatus(AgentToolCall toolCall) {
229-
if (toolCall == null || StringUtils.isEmpty(toolCall.getProgressMessage())) {
229+
if (toolCall == null
230+
|| (StringUtils.isBlank(toolCall.getProgressMessage()) && StringUtils.isBlank(toolCall.getError()))) {
230231
return;
231232
}
232233

@@ -268,6 +269,11 @@ public void appendToolCallStatus(AgentToolCall toolCall) {
268269
break;
269270
case "error":
270271
statusLabel.setErrorStatus();
272+
String errorText = StringUtils.isNotBlank(toolCall.getError()) ? toolCall.getError()
273+
: toolCall.getProgressMessage();
274+
if (StringUtils.isNotBlank(errorText)) {
275+
statusLabel.setText(errorText);
276+
}
271277
break;
272278
default:
273279
statusLabel.setErrorStatus();

0 commit comments

Comments
 (0)