Azure / azure-sdk-for-java

This repository is for active development of the Azure SDK for Java. For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/java/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-java.
MIT License
2.36k stars 2k forks source link

[BUG] openai ChatRequestUserMessage deserialization loses internal state #42882

Open fairjm opened 2 weeks ago

fairjm commented 2 weeks ago

Describe the bug ChatRequestUserMessage loses its internal state during deserialization. The following fields are set to null:

    private final String stringContent;

    private final List<ChatMessageContentItem> chatMessageContentItems;

When these fields are null, subsequent requests fail.

To Reproduce

    @Test
    public void test() throws IOException {
        String userMessage = """
                    {
                      "role": "user",
                      "content": "this is test message"
                    }
                """;

        ChatRequestUserMessage u = BinaryData.fromString(userMessage).toObject(new TypeReference<>() {
        });
        System.out.println(u.toJsonString());
    }

it will output:

{"role":"user"}

Code Snippet Add the code snippet that causes the issue.

Expected behavior

Screenshots If applicable, add screenshots to help explain your problem.

Setup (please complete the following information):

If you suspect a dependency version mismatch (e.g. you see NoClassDefFoundError, NoSuchMethodError or similar), please check out Troubleshoot dependency version conflict article first. If it doesn't provide solution for the problem, please provide:

Additional context

    @Override
    public JsonWriter toJson(JsonWriter jsonWriter) throws IOException {
        jsonWriter.writeStartObject();
        jsonWriter.writeStringField("role", this.role == null ? null : this.role.toString());
        if (stringContent != null) {
            jsonWriter.writeStringField("content", stringContent);
        } else if (chatMessageContentItems != null) {
            jsonWriter.writeArrayField("content", chatMessageContentItems, JsonWriter::writeJson);
        }
        //  or set the content back here?👉
        jsonWriter.writeStringField("name", this.name);
        return jsonWriter.writeEndObject();
    }

    public static ChatRequestUserMessage fromJson(JsonReader jsonReader) throws IOException {
        return jsonReader.readObject(reader -> {
            BinaryData content = null;
            ChatRole role = ChatRole.USER;
            String name = null;
            while (reader.nextToken() != JsonToken.END_OBJECT) {
                String fieldName = reader.getFieldName();
                reader.nextToken();
                if ("content".equals(fieldName)) {

                    // here 👇 forget to set the stringContent or chatMessageContentItems
                    if (reader.currentToken() == JsonToken.STRING) {
                        content = BinaryData.fromString(reader.getString());
                    } else if (reader.currentToken() == JsonToken.START_ARRAY) {
                        content = BinaryData.fromObject(
                            reader.readArray(arrayReader -> arrayReader.readObject(ChatMessageContentItem::fromJson)));
                    } else {
                        throw new IllegalStateException("Unexpected 'content' type found when deserializing"
                            + " ChatRequestUserMessage JSON object: " + reader.currentToken());
                    }
                } else if ("role".equals(fieldName)) {
                    role = ChatRole.fromString(reader.getString());
                } else if ("name".equals(fieldName)) {
                    name = reader.getString();
                } else {
                    reader.skipChildren();
                }
            }
            ChatRequestUserMessage deserializedChatRequestUserMessage = new ChatRequestUserMessage(content);
            deserializedChatRequestUserMessage.role = role;
            deserializedChatRequestUserMessage.name = name;
            return deserializedChatRequestUserMessage;
        });

During serialization, the internal stringContent and chatMessageContentItems fields are used. However, during deserialization, these internal states are not set properly. As a result, after one round of deserialization and serialization, the content is completely lost. This bug has existed for a long time.

Information Checklist Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

fairjm commented 2 weeks ago

I've noticed that ChatRequestSystemMessage, ChatRequestAssistantMessage and other similar message types all lose their internal state during deserialization. Is the API design of ChatRequest messages not intended for deserialization?

alzimmermsft commented 2 weeks ago

Thanks for reporting this @fairjm, @mssfang or @jpalvarezl could you looking into this and response