diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/llm/ChatCompletionService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/llm/ChatCompletionService.java index 96e76b3c168..2414f251e8e 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/llm/ChatCompletionService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/llm/ChatCompletionService.java @@ -4,6 +4,8 @@ import com.comet.opik.infrastructure.LlmProviderClientConfig; import com.comet.opik.utils.ChunkedOutputHandlers; import com.google.common.base.Throwables; +import dev.langchain4j.exception.NonRetriableException; +import dev.langchain4j.exception.UnsupportedFeatureException; import dev.langchain4j.internal.RetryUtils; import dev.langchain4j.model.chat.request.ChatRequest; import dev.langchain4j.model.chat.response.ChatResponse; @@ -20,11 +22,13 @@ import lombok.NonNull; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.exception.ExceptionUtils; import ru.vyarus.dropwizard.guice.module.yaml.bind.Config; import java.net.ConnectException; import java.nio.channels.ClosedChannelException; import java.util.Optional; +import java.util.concurrent.Callable; import java.util.function.Consumer; import static jakarta.ws.rs.core.Response.Status.Family.familyOf; @@ -33,6 +37,7 @@ @Slf4j public class ChatCompletionService { public static final String UNEXPECTED_ERROR_CALLING_LLM_PROVIDER = "Unexpected error calling LLM provider"; + public static final String UNSUPPORTED_FEATURE_CALLING_LLM_PROVIDER = "Unsupported feature for the selected LLM provider"; public static final String ERROR_EMPTY_MESSAGES = "messages cannot be empty"; private final LlmProviderClientConfig llmProviderClientConfig; @@ -58,8 +63,11 @@ public ChatCompletionResponse create(@NonNull ChatCompletionRequest rawRequest, ChatCompletionResponse chatCompletionResponse; try { log.info("Creating chat completions, workspaceId '{}', model '{}'", workspaceId, request.model()); - chatCompletionResponse = retryPolicy.withRetry(() -> llmProviderClient.generate(request, workspaceId)); + chatCompletionResponse = retryPolicy.withRetry( + () -> failFastOnUnsupportedFeature(() -> llmProviderClient.generate(request, workspaceId))); } catch (RuntimeException runtimeException) { + failIfUnsupportedFeature(runtimeException); + Optional providerError = llmProviderClient.getLlmProviderError(runtimeException); providerError @@ -82,13 +90,27 @@ public void createAndStreamResponse( log.info("Creating and streaming chat completions, workspaceId '{}', model '{}'", workspaceId, request.model()); var llmProviderClient = llmProviderFactory.getService(workspaceId, request.model()); + var errorHandler = getErrorHandler(handlers, llmProviderClient); - llmProviderClient.generateStream( - request, - workspaceId, - handlers::handleMessage, - handlers::handleClose, - getErrorHandler(handlers, llmProviderClient)); + try { + llmProviderClient.generateStream( + request, + workspaceId, + handlers::handleMessage, + handlers::handleClose, + errorHandler); + } catch (UnsupportedFeatureException unsupportedFeature) { + // Streaming clients get one contract: HTTP 200 with the error delivered in-stream. VertexAI and Gemini + // already guarantee that by catching everything inside their own boundedElastic task, but + // OpenAiResponses, OpenAI, CustomLlm and Anthropic run inline, so an unsupported feature raised before + // the provider engages would otherwise escape as an HTTP status and break the contract for those + // providers only. Caught by exact type rather than RuntimeException: no retry policy wraps this call, so + // these arrive unwrapped, and everything else keeps propagating to the resource layer untouched. + // BadRequestException is deliberately NOT caught: LlmProviderAnthropic.generateStream validates messages + // inline and throws it, and that must stay a real HTTP 400. + errorHandler.accept(unsupportedFeature); + return; + } log.info("Created and streaming chat completions, workspaceId '{}', model '{}'", workspaceId, request.model()); @@ -104,11 +126,13 @@ public ChatResponse scoreTrace(@NonNull ChatRequest chatRequest, log.info("Initiating chat with model '{}' expecting structured response, workspaceId '{}'", modelParameters.name(), workspaceId); chatResponse = retryPolicy - .withRetry(() -> languageModelClient.chat(chatRequest)); + .withRetry(() -> failFastOnUnsupportedFeature(() -> languageModelClient.chat(chatRequest))); log.info("Completed chat with model '{}' expecting structured response, workspaceId '{}'", modelParameters.name(), workspaceId); return chatResponse; } catch (RuntimeException runtimeException) { + failIfUnsupportedFeature(runtimeException); + LlmProviderService provider = llmProviderFactory.getService(workspaceId, modelParameters.name()); Optional providerError = provider.getLlmProviderError(runtimeException); @@ -121,6 +145,76 @@ public ChatResponse scoreTrace(@NonNull ChatRequest chatRequest, } } + /** + * {@link UnsupportedFeatureException} extends {@code LangChain4jException}, not {@code NonRetriableException}, so + * {@code RetryPolicy.withRetry} treats it like any transient failure and burns the whole retry budget (plus its + * backoff delays) on a call that can never succeed. Re-throwing it as {@link NonRetriableException} makes + * {@code withRetry} give up on the first attempt while leaving genuinely transient provider errors retryable. The + * original exception is kept as the cause, so {@link #failIfUnsupportedFeature} still recognises it downstream. + */ + private T failFastOnUnsupportedFeature(Callable action) throws Exception { + try { + return action.call(); + } catch (RuntimeException runtimeException) { + if (findUnsupportedFeature(runtimeException).isPresent()) { + throw new NonRetriableException(runtimeException); + } + throw runtimeException; + } + } + + /** + * langchain4j raises {@link UnsupportedFeatureException} when the request asks for a capability the selected + * provider does not implement — e.g. {@code ToolChoice.REQUIRED} against Vertex AI Gemini. The provider is never + * reached, so {@code getLlmProviderError} has nothing to map and the call used to surface as a 500. That is + * misleading on two counts: nothing failed server-side, and no amount of retrying can make it succeed. Report it + * as a 400 so clients get an actionable error and the online-scoring consumers treat it as terminal instead of + * burning their retry budget on it. + */ + private void failIfUnsupportedFeature(RuntimeException runtimeException) { + var unsupportedFeature = findUnsupportedFeature(runtimeException); + if (unsupportedFeature.isEmpty()) { + return; + } + + var message = buildUnsupportedFeatureMessage(unsupportedFeature.get()); + // Logged without the throwable: this is an expected, deterministic client error, and at production volumes a + // stack trace per rejection buries the genuine provider failures. + log.warn(message); + // The message is carried as an ErrorMessage entity, not just on the exception: Jersey renders + // WebApplicationException via its Response, so a message-only constructor would return a bodiless 400 and the + // caller would never learn which capability was rejected. + throw new BadRequestException( + message, + Response.status(Response.Status.BAD_REQUEST) + .entity(new ErrorMessage(Response.Status.BAD_REQUEST.getStatusCode(), message)) + .build(), + runtimeException); + } + + /** + * Built from the {@link UnsupportedFeatureException}'s own message rather than the chain's root cause, so the + * client is told which capability was rejected and nothing deeper in the chain can leak into the response. + */ + private String buildUnsupportedFeatureMessage(UnsupportedFeatureException unsupportedFeature) { + String detail = unsupportedFeature.getMessage(); + return StringUtils.isNotBlank(detail) + ? UNSUPPORTED_FEATURE_CALLING_LLM_PROVIDER + ": " + detail + : UNSUPPORTED_FEATURE_CALLING_LLM_PROVIDER; + } + + /** + * Walks the cause chain, so it matches whether the exception is thrown bare, wrapped by a provider client, or + * re-thrown by {@link #failFastOnUnsupportedFeature}. {@code ExceptionUtils} stops at the first already-visited + * throwable, so a self-referencing cause chain terminates rather than looping. + */ + private Optional findUnsupportedFeature(Throwable throwable) { + return ExceptionUtils.getThrowableList(throwable).stream() + .filter(UnsupportedFeatureException.class::isInstance) + .map(UnsupportedFeatureException.class::cast) + .findFirst(); + } + private void failHandlingLLMProviderError(RuntimeException runtimeException, ErrorMessage llmProviderError) { log.warn(UNEXPECTED_ERROR_CALLING_LLM_PROVIDER, runtimeException); @@ -141,6 +235,16 @@ private RetryUtils.RetryPolicy newRetryPolicy() { private Consumer getErrorHandler(ChunkedOutputHandlers handlers, LlmProviderService llmProviderClient) { return throwable -> { + // Checked before the provider-error mapper so the classification matches create() and scoreTrace(): if a + // provider envelope and an unsupported feature ever collide, the deterministic capability failure wins. + var unsupportedFeature = findUnsupportedFeature(throwable); + if (unsupportedFeature.isPresent()) { + var message = buildUnsupportedFeatureMessage(unsupportedFeature.get()); + log.warn(message); + handlers.handleError(new ErrorMessage(Response.Status.BAD_REQUEST.getStatusCode(), message)); + return; + } + Optional providerError = llmProviderClient.getLlmProviderError(throwable); if (providerError.isPresent()) { diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ChatCompletionsResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ChatCompletionsResourceTest.java index c80506fb405..5c54db2a434 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ChatCompletionsResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ChatCompletionsResourceTest.java @@ -309,6 +309,31 @@ void createAndStreamResponseReturnsBadRequestWhenNoModel(String model) { assertThat(errorMessage.getMessage()) .containsIgnoringCase(ERROR_MODEL_NOT_SUPPORTED.formatted(model)); } + + @Test + void createAndStreamResponseReturnsBadRequestWhenNoMessages() { + var workspaceName = RandomStringUtils.randomAlphanumeric(20); + var workspaceId = UUID.randomUUID().toString(); + mockTargetWorkspace(workspaceName, workspaceId); + createLlmProviderApiKey(workspaceName, LlmProvider.ANTHROPIC, UUID.randomUUID().toString()); + + // The streaming twin of createAnthropicValidateMandatoryFields, which pins stream(false) and so cannot + // catch a regression on this path. LlmProviderAnthropic.generateStream validates messages inline before + // any provider I/O, which is why a throwaway API key still reaches the rejection. This must stay a real + // HTTP 400 rather than a 200 carrying the error inside the SSE body: nothing upstream validates messages, + // and callers branch on the status. + var request = podamFactory.manufacturePojo(ChatCompletionRequest.Builder.class) + .stream(true) + .model(AnthropicModelName.CLAUDE_SONNET_3_7.toString()) + .maxCompletionTokens(100) + .build(); + + var errorMessage = chatCompletionsClient.createAndStreamError(API_KEY, workspaceName, request, + HttpStatus.SC_BAD_REQUEST); + + assertThat(errorMessage.getCode()).isEqualTo(HttpStatus.SC_BAD_REQUEST); + assertThat(errorMessage.getMessage()).containsIgnoringCase(ERROR_EMPTY_MESSAGES); + } } @ParameterizedTest diff --git a/apps/opik-backend/src/test/java/com/comet/opik/domain/llm/ChatCompletionServiceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/domain/llm/ChatCompletionServiceTest.java index 6363c3b74cf..eb703672f47 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/domain/llm/ChatCompletionServiceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/domain/llm/ChatCompletionServiceTest.java @@ -1,10 +1,17 @@ package com.comet.opik.domain.llm; +import com.comet.opik.api.evaluators.LlmAsJudgeModelParameters; import com.comet.opik.infrastructure.LlmProviderClientConfig; import com.comet.opik.podam.PodamFactoryUtils; +import com.comet.opik.utils.ChunkedOutputHandlers; +import dev.langchain4j.data.message.UserMessage; +import dev.langchain4j.exception.UnsupportedFeatureException; +import dev.langchain4j.model.chat.ChatModel; +import dev.langchain4j.model.chat.request.ChatRequest; import dev.langchain4j.model.openai.internal.chat.ChatCompletionRequest; import dev.langchain4j.model.openai.internal.chat.ChatCompletionResponse; import io.dropwizard.jersey.errors.ErrorMessage; +import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.InternalServerErrorException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -14,6 +21,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import uk.co.jemos.podam.api.PodamFactory; @@ -21,12 +29,22 @@ import java.net.ConnectException; import java.nio.channels.ClosedChannelException; import java.util.Optional; +import java.util.function.Consumer; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.catchThrowable; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.atLeast; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -44,6 +62,9 @@ class ChatCompletionServiceTest { @Mock private LlmProviderService llmProviderService; + @Mock + private ChatModel chatModel; + private ChatCompletionService chatCompletionService; @BeforeEach @@ -201,6 +222,307 @@ void create__whenSuccessful__thenReturnResponse() { } } + @Nested + @DisplayName("Unsupported Feature Handling:") + class UnsupportedFeatureHandling { + + private static final String UNSUPPORTED_FEATURE_MESSAGE = "ToolChoice.REQUIRED is not supported yet by this model provider"; + + private static Stream unsupportedFeatureExceptionCases() { + return Stream.of( + Arguments.of( + "thrown directly", + new UnsupportedFeatureException(UNSUPPORTED_FEATURE_MESSAGE)), + Arguments.of( + "wrapped in a RuntimeException", + new RuntimeException("Retry wrapper", + new UnsupportedFeatureException(UNSUPPORTED_FEATURE_MESSAGE)))); + } + + @ParameterizedTest(name = "when UnsupportedFeatureException is {0}, then throw BadRequestException") + @MethodSource("unsupportedFeatureExceptionCases") + @DisplayName("create should map unsupported features to 400, not 500") + void create__whenUnsupportedFeatureException__thenThrowBadRequest( + String testName, RuntimeException runtimeException) { + // Given + var request = podamFactory.manufacturePojo(ChatCompletionRequest.class); + var workspaceId = "test-workspace-id"; + + when(llmProviderFactory.getService(anyString(), anyString())).thenReturn(llmProviderService); + when(llmProviderService.generate(any(), anyString())).thenThrow(runtimeException); + + // When + var thrown = catchThrowable(() -> chatCompletionService.create(request, workspaceId)); + + // Then + assertThat(thrown) + .isInstanceOf(BadRequestException.class) + .hasMessageContaining("Unsupported feature for the selected LLM provider") + .hasMessageContaining(UNSUPPORTED_FEATURE_MESSAGE); + assertThat(((BadRequestException) thrown).getResponse().getStatus()).isEqualTo(400); + } + + @ParameterizedTest(name = "when UnsupportedFeatureException is {0}, then throw BadRequestException") + @MethodSource("unsupportedFeatureExceptionCases") + @DisplayName("scoreTrace should map unsupported features to 400, not 500") + void scoreTrace__whenUnsupportedFeatureException__thenThrowBadRequest( + String testName, RuntimeException runtimeException) { + // Given + var chatRequest = ChatRequest.builder().messages(UserMessage.from("score this")).build(); + var modelParameters = LlmAsJudgeModelParameters.builder() + .name("vertex_ai/gemini-3.1-flash-lite") + .temperature(0.0) + .build(); + var workspaceId = "test-workspace-id"; + + when(llmProviderFactory.getLanguageModel(anyString(), any())).thenReturn(chatModel); + when(chatModel.chat(any(ChatRequest.class))).thenThrow(runtimeException); + + // When + var thrown = catchThrowable( + () -> chatCompletionService.scoreTrace(chatRequest, modelParameters, workspaceId)); + + // Then + assertThat(thrown) + .isInstanceOf(BadRequestException.class) + .hasMessageContaining("Unsupported feature for the selected LLM provider") + .hasMessageContaining(UNSUPPORTED_FEATURE_MESSAGE); + assertThat(((BadRequestException) thrown).getResponse().getStatus()).isEqualTo(400); + } + + @Test + @DisplayName("scoreTrace should not consult the provider error mapper for unsupported features") + void scoreTrace__whenUnsupportedFeatureException__thenSkipProviderErrorLookup() { + // Given + var chatRequest = ChatRequest.builder().messages(UserMessage.from("score this")).build(); + var modelParameters = LlmAsJudgeModelParameters.builder() + .name("vertex_ai/gemini-3.1-flash-lite") + .temperature(0.0) + .build(); + var workspaceId = "test-workspace-id"; + + when(llmProviderFactory.getLanguageModel(anyString(), any())).thenReturn(chatModel); + when(chatModel.chat(any(ChatRequest.class))) + .thenThrow(new UnsupportedFeatureException(UNSUPPORTED_FEATURE_MESSAGE)); + + // When & Then + assertThatThrownBy(() -> chatCompletionService.scoreTrace(chatRequest, modelParameters, workspaceId)) + .isInstanceOf(BadRequestException.class); + + // The provider was never reached, so there is no provider error to map. + verify(llmProviderFactory, never()).getService(anyString(), anyString()); + verify(llmProviderService, never()).getLlmProviderError(any()); + } + + @Test + @DisplayName("the 400 response body carries the unsupported-feature message") + void create__whenUnsupportedFeatureException__thenResponseBodyCarriesMessage() { + // Given + var request = podamFactory.manufacturePojo(ChatCompletionRequest.class); + var workspaceId = "test-workspace-id"; + + when(llmProviderFactory.getService(anyString(), anyString())).thenReturn(llmProviderService); + when(llmProviderService.generate(any(), anyString())) + .thenThrow(new UnsupportedFeatureException(UNSUPPORTED_FEATURE_MESSAGE)); + + // When + var thrown = (BadRequestException) catchThrowable( + () -> chatCompletionService.create(request, workspaceId)); + + // Then — Jersey renders the Response, so the entity is what the caller actually receives + var response = thrown.getResponse(); + assertThat(response.getStatus()).isEqualTo(400); + assertThat(response.getEntity()).isInstanceOf(ErrorMessage.class); + + var errorMessage = (ErrorMessage) response.getEntity(); + assertThat(errorMessage.getCode()).isEqualTo(400); + assertThat(errorMessage.getMessage()) + .isEqualTo("Unsupported feature for the selected LLM provider: " + UNSUPPORTED_FEATURE_MESSAGE); + } + + @Test + @DisplayName("only the unsupported-feature message is exposed, not deeper causes") + void create__whenUnsupportedFeatureWrapsAnotherCause__thenDeeperDetailNotExposed() { + // Given — a deeper root cause that must not reach the client + var request = podamFactory.manufacturePojo(ChatCompletionRequest.class); + var workspaceId = "test-workspace-id"; + var wrapped = new RuntimeException("internal wiring detail", + new UnsupportedFeatureException(UNSUPPORTED_FEATURE_MESSAGE)); + + when(llmProviderFactory.getService(anyString(), anyString())).thenReturn(llmProviderService); + when(llmProviderService.generate(any(), anyString())).thenThrow(wrapped); + + // When + var thrown = (BadRequestException) catchThrowable( + () -> chatCompletionService.create(request, workspaceId)); + + // Then + var errorMessage = (ErrorMessage) thrown.getResponse().getEntity(); + assertThat(errorMessage.getMessage()) + .isEqualTo("Unsupported feature for the selected LLM provider: " + UNSUPPORTED_FEATURE_MESSAGE) + .doesNotContain("internal wiring detail"); + } + + @Test + @DisplayName("unsupported features must not consume the provider retry budget") + void create__whenUnsupportedFeatureException__thenNotRetried() { + // Given — a policy that would retry, unlike the single-attempt default used by the other tests + var request = podamFactory.manufacturePojo(ChatCompletionRequest.class); + var workspaceId = "test-workspace-id"; + + when(llmProviderClientConfig.getMaxAttempts()).thenReturn(3); + var retryingService = new ChatCompletionService(llmProviderClientConfig, llmProviderFactory); + + when(llmProviderFactory.getService(anyString(), anyString())).thenReturn(llmProviderService); + when(llmProviderService.generate(any(), anyString())) + .thenThrow(new UnsupportedFeatureException(UNSUPPORTED_FEATURE_MESSAGE)); + + // When & Then + assertThatThrownBy(() -> retryingService.create(request, workspaceId)) + .isInstanceOf(BadRequestException.class); + + // UnsupportedFeatureException extends LangChain4jException, not NonRetriableException, so without the + // fail-fast wrapper langchain4j's RetryPolicy would retry a call that can never succeed. + verify(llmProviderService, times(1)).generate(any(), anyString()); + } + + @Test + @DisplayName("transient failures must still be retried") + void create__whenTransientException__thenStillRetried() { + // Given + var request = podamFactory.manufacturePojo(ChatCompletionRequest.class); + var workspaceId = "test-workspace-id"; + + when(llmProviderClientConfig.getMaxAttempts()).thenReturn(3); + var retryingService = new ChatCompletionService(llmProviderClientConfig, llmProviderFactory); + + when(llmProviderFactory.getService(anyString(), anyString())).thenReturn(llmProviderService); + when(llmProviderService.generate(any(), anyString())).thenThrow(new RuntimeException("transient")); + when(llmProviderService.getLlmProviderError(any())).thenReturn(Optional.empty()); + + // When & Then + assertThatThrownBy(() -> retryingService.create(request, workspaceId)) + .isInstanceOf(InternalServerErrorException.class); + + verify(llmProviderService, atLeast(2)).generate(any(), anyString()); + } + + @Test + @DisplayName("streaming gives unsupported features precedence over a provider error envelope") + void createAndStreamResponse__whenUnsupportedFeatureAndProviderError__thenReportBadRequest() { + // Given — the provider mapper would happily classify this throwable, but the capability failure wins + var request = podamFactory.manufacturePojo(ChatCompletionRequest.class); + var workspaceId = "test-workspace-id"; + var handlers = mock(ChunkedOutputHandlers.class); + var unsupported = new UnsupportedFeatureException(UNSUPPORTED_FEATURE_MESSAGE); + + when(llmProviderFactory.getService(anyString(), anyString())).thenReturn(llmProviderService); + lenient().when(llmProviderService.getLlmProviderError(any())) + .thenReturn(Optional.of(new ErrorMessage(503, "provider says unavailable"))); + doAnswer(invocation -> { + Consumer errorHandler = invocation.getArgument(4); + errorHandler.accept(unsupported); + return null; + }).when(llmProviderService).generateStream(any(), anyString(), any(), any(), any()); + + // When + chatCompletionService.createAndStreamResponse(request, workspaceId, handlers); + + // Then + var errorCaptor = ArgumentCaptor.forClass(ErrorMessage.class); + verify(handlers).handleError(errorCaptor.capture()); + assertThat(errorCaptor.getValue().getCode()).isEqualTo(400); + assertThat(errorCaptor.getValue().getMessage()) + .contains("Unsupported feature for the selected LLM provider") + .contains(UNSUPPORTED_FEATURE_MESSAGE); + } + + @Test + @DisplayName("a synchronous BadRequestException still escapes as a real HTTP status") + void createAndStreamResponse__whenGenerateStreamThrowsBadRequest__thenPropagates() { + // Given — LlmProviderAnthropic.generateStream calls validateRequest inline and throws this before any + // provider I/O. Nothing upstream validates messages: ChatCompletionsResource only checks the model, and + // ChatCompletionRequest is langchain4j's class, so @Valid contributes no constraints. Swallowing it into + // the stream would turn a malformed request into an HTTP 200 for callers branching on status. + var request = podamFactory.manufacturePojo(ChatCompletionRequest.class); + var workspaceId = "test-workspace-id"; + var handlers = mock(ChunkedOutputHandlers.class); + + when(llmProviderFactory.getService(anyString(), anyString())).thenReturn(llmProviderService); + doThrow(new BadRequestException(ChatCompletionService.ERROR_EMPTY_MESSAGES)) + .when(llmProviderService).generateStream(any(), anyString(), any(), any(), any()); + + // When & Then — createAndStreamResponse runs before Response.ok() is built, so nothing is committed yet + // and this surfaces as a genuine 400 + assertThatThrownBy(() -> chatCompletionService.createAndStreamResponse(request, workspaceId, handlers)) + .isInstanceOf(BadRequestException.class) + .hasMessageContaining(ChatCompletionService.ERROR_EMPTY_MESSAGES); + + verify(handlers, never()).handleError(any()); + } + + @Test + @DisplayName("a synchronously thrown UnsupportedFeatureException is delivered in-stream as a code-400 ErrorMessage, leaving the HTTP response 200") + void createAndStreamResponse__whenGenerateStreamThrowsUnsupportedFeature__thenErrorStreamed() { + // Given — OpenAiResponses and friends run inline, so this escapes generateStream instead of reaching the + // error callback the way VertexAI and Gemini do + var request = podamFactory.manufacturePojo(ChatCompletionRequest.class); + var workspaceId = "test-workspace-id"; + var handlers = mock(ChunkedOutputHandlers.class); + + when(llmProviderFactory.getService(anyString(), anyString())).thenReturn(llmProviderService); + doThrow(new UnsupportedFeatureException(UNSUPPORTED_FEATURE_MESSAGE)) + .when(llmProviderService).generateStream(any(), anyString(), any(), any(), any()); + + // When — must not escape, so the resource still returns 200 with the stream + chatCompletionService.createAndStreamResponse(request, workspaceId, handlers); + + // Then + var errorCaptor = ArgumentCaptor.forClass(ErrorMessage.class); + verify(handlers).handleError(errorCaptor.capture()); + assertThat(errorCaptor.getValue().getCode()).isEqualTo(400); + assertThat(errorCaptor.getValue().getMessage()) + .isEqualTo("Unsupported feature for the selected LLM provider: " + UNSUPPORTED_FEATURE_MESSAGE); + } + + @Test + @DisplayName("unrelated synchronous failures still propagate to the resource layer") + void createAndStreamResponse__whenGenerateStreamThrowsUnrelated__thenPropagates() { + // Given + var request = podamFactory.manufacturePojo(ChatCompletionRequest.class); + var workspaceId = "test-workspace-id"; + var handlers = mock(ChunkedOutputHandlers.class); + + when(llmProviderFactory.getService(anyString(), anyString())).thenReturn(llmProviderService); + doThrow(new IllegalStateException("connection pool exhausted")) + .when(llmProviderService).generateStream(any(), anyString(), any(), any(), any()); + + // When & Then — not a client error, so it keeps its existing behaviour rather than becoming a 200 + assertThatThrownBy(() -> chatCompletionService.createAndStreamResponse(request, workspaceId, handlers)) + .isInstanceOf(IllegalStateException.class) + .hasMessage("connection pool exhausted"); + + verify(handlers, never()).handleError(any()); + } + + @Test + @DisplayName("unrelated runtime exceptions should still produce a 500") + void create__whenUnrelatedException__thenStillThrowInternalServerError() { + // Given + var request = podamFactory.manufacturePojo(ChatCompletionRequest.class); + var workspaceId = "test-workspace-id"; + + when(llmProviderFactory.getService(anyString(), anyString())).thenReturn(llmProviderService); + when(llmProviderService.generate(any(), anyString())).thenThrow(new RuntimeException("boom")); + when(llmProviderService.getLlmProviderError(any())).thenReturn(Optional.empty()); + + // When & Then + assertThatThrownBy(() -> chatCompletionService.create(request, workspaceId)) + .isInstanceOf(InternalServerErrorException.class) + .hasMessageContaining("Unexpected error calling LLM provider"); + } + } + @Nested @DisplayName("Error Message Construction:") class ErrorMessageConstruction {