Copilot commented on code in PR #993:
URL: https://github.com/apache/dubbo-go-samples/pull/993#discussion_r2605350477


##########
h3/README.md:
##########
@@ -0,0 +1,152 @@
+# H3 (HTTP/3) for dubbo-go
+
+English | [中文](README_CN.md)
+
+This example demonstrates how to use dubbo-go with HTTP/3 protocol support via 
the Triple protocol. It shows how to enable HTTP/3 for high-performance 
communication between Go and Java services using TLS for secure connections.
+
+## Contents
+
+- go-server/cmd/main.go - is the main definition of the service, handler and 
rpc server with HTTP/3 support
+- go-client/cmd/main.go - is the rpc client with HTTP/3 support
+- java-server/src/main/java/org/apache/dubbo/samples/h3/H3ServerApp.java - is 
the Java server with HTTP/3 support
+- java-client/src/main/java/org/apache/dubbo/samples/h3/H3ClientApp.java - is 
the Java client with HTTP/3 support
+- proto - contains the protobuf definition of the API
+- x509 - contains TLS certificates and keys for secure connections
+
+## Key Features
+
+- **HTTP/3 Protocol Support**: Uses QUIC transport for faster, more reliable 
connections
+- **Cross-Language Interoperability**: Demonstrates Go and Java 
interoperability with HTTP/3
+- **TLS Encryption**: Secure communication with client and server certificates
+- **Triple Protocol**: Built on Apache Dubbo's Triple protocol with HTTP/3 
enabled
+
+## How to run
+
+### Prerequisites
+1. Install `protoc` [version3][]
+   Please refer to [Protocol Buffer Compiler Installation][].
+
+2. Install `protoc-gen-go` and `protoc-gen-triple`
+   Install the version of your choice of protoc-gen-go. here use the latest 
version as example:
+
+    ```shell
+    go install google.golang.org/protobuf/cmd/[email protected]
+    ```
+   
+    Install the latest version of protoc-gen-triple:
+
+    ```shell
+    go install github.com/dubbogo/protoc-gen-go-triple/[email protected]
+    ```
+
+3. Generate stub code
+
+    Generate related stub code with protoc-gen-go and protoc-gen-triple:
+
+    ```shell
+    protoc --go_out=. --go_opt=paths=source_relative --go-triple_out=. 
--go-triple_opt=paths=source_relative ./proto/greet.proto
+    ```
+   
+4. Install `Maven` [Maven][]
+
+### Run Golang server
+```shell
+cd go-server/cmd
+go run main.go
+```
+
+Test server works as expected:
+```shell
+curl -k \
+    --header "Content-Type: application/json" \
+    --data '{"name": "Dubbo"}' \
+    https://localhost:20000/greet.GreetService/Greet
+```
+
+### Run Golang client
+```shell
+cd go-client/cmd
+go run main.go
+```
+
+### Run Java server
+
+Build all Java modules from the root directory:
+```shell
+mvn clean compile
+```
+
+Run the Java server:
+
+**On Linux/Mac/Git Bash:**
+```shell
+cd java-server
+mvn exec:java -Dexec.mainClass=org.apache.dubbo.samples.h3.H3ServerApp
+```
+
+**On Windows PowerShell:**
+```powershell
+cd java-server
+mvn exec:java "-Dexec.mainClass=org.apache.dubbo.samples.h3.H3ServerApp"
+```
+
+**Or use the provided script (Linux/Mac):**
+```shell
+cd java-server
+./run.sh
+```
+
+Test server works as expected:
+```shell
+curl -k \
+    --header "Content-Type: application/json" \
+    --data '{"name": "Dubbo"}' \
+    https://localhost:20000/greet.GreetService/Greet
+```
+
+### Run Java client
+
+Run the Java client:
+
+**On Linux/Mac/Git Bash:**
+```shell
+cd java-client
+mvn exec:java -Dexec.mainClass=org.apache.dubbo.samples.h3.H3ClientApp
+```
+
+**On Windows PowerShell:**
+```powershell
+cd java-client
+mvn exec:java "-Dexec.mainClass=org.apache.dubbo.samples.h3.H3ClientApp"
+```
+
+**Or use the provided script (Linux/Mac):**
+```shell
+cd java-client
+./run.sh

Review Comment:
   Incorrect directory referenced in the script usage instructions. The 
instructions say to run `./run.sh` from the `java-client` directory (line 123), 
but then reference `java-client` directory on line 125. This should remain 
`java-client` to match the context, not reference a different directory.
   
   Should be:
   ```shell
   cd java-client
   ./run.sh
   ```



##########
h3/proto/greet.proto:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+syntax = "proto3";
+
+package greet;
+
+option go_package = "github.com/apache/dubbo-go-samples/h3/proto;greet";
+
+message GreetRequest {
+  string name = 1;
+}
+
+message GreetResponse {
+  string greeting = 1;
+}
+
+service GreetService {
+  rpc Greet(GreetRequest) returns (GreetResponse) {}
+}

Review Comment:
   The ordering of message definitions and service definitions is inconsistent 
with the Java proto files. In the Java proto files 
(java-server/src/main/proto/greet.proto and 
java-client/src/main/proto/greet.proto), the service definition comes before 
the message definitions (lines 25-27, then 29-35), while here the messages come 
before the service (lines 24-30, then 32-34). 
   
   For consistency and maintainability, all proto files should follow the same 
ordering convention.



##########
h3/go-client/cmd/main.go:
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package main
+
+import (
+       "context"
+       "time"
+)
+
+import (
+       "dubbo.apache.org/dubbo-go/v3/client"
+       _ "dubbo.apache.org/dubbo-go/v3/imports"
+       "dubbo.apache.org/dubbo-go/v3/protocol"
+       "dubbo.apache.org/dubbo-go/v3/protocol/triple"
+       "dubbo.apache.org/dubbo-go/v3/tls"
+
+       "github.com/dubbogo/gost/log/logger"
+)
+
+import (
+       greet "github.com/apache/dubbo-go-samples/h3/proto"
+)
+
+func main() {
+       logger.SetLoggerLevel("debug")
+
+       cli, err := client.NewClient(
+               client.WithClientURL("127.0.0.1:20000"),
+               client.WithClientTLSOption(
+                       tls.WithCACertFile("../../x509/server_ca_cert.pem"),
+                       tls.WithCertFile("../../x509/server2_cert.pem"),
+                       tls.WithKeyFile("../../x509/server2_key_pkcs8.pem"),
+                       tls.WithServerName("dubbogo.test.example.com"),
+               ),
+               // Enable HTTP/3 support on client side
+               // This configures the client to use dualTransport which 
supports
+               // both HTTP/2 and HTTP/3 with Alt-Svc negotiation
+               
client.WithClientProtocol(protocol.WithTriple(triple.Http3Enable())),
+       )
+       if err != nil {
+               logger.Fatalf("failed to create client: %v", err)
+               return
+       }
+
+       svc, err := greet.NewGreetService(cli)
+       if err != nil {
+               logger.Fatalf("failed to create greet service: %v", err)
+       }
+
+       for i := 0; i < 3; i += 1 {
+               ctx, cancel := context.WithTimeout(context.Background(), 
time.Second)
+
+               defer cancel()
+
+               resp, err := svc.Greet(ctx, &greet.GreetRequest{Name: "Go 
Client"})
+               if err != nil {
+                       logger.Errorf("failed to greet: %v", err)
+               }
+               logger.Infof("Greet response: %s", resp.Greeting)
+

Review Comment:
   The `defer cancel()` is placed inside the loop, which means all three 
context cancellations will be deferred until the function exits. This could 
lead to resource leaks as the contexts won't be canceled immediately after each 
request completes.
   
   Move the `defer cancel()` statement outside of the loop scope or call 
`cancel()` explicitly after each request:
   
   ```go
   for i := 0; i < 3; i += 1 {
       ctx, cancel := context.WithTimeout(context.Background(), time.Second)
       
       resp, err := svc.Greet(ctx, &greet.GreetRequest{Name: "Go Client"})
       if err != nil {
           logger.Errorf("failed to greet: %v", err)
       } else {
           logger.Infof("Greet response: %s", resp.Greeting)
       }
       
       cancel() // Cancel immediately after use
       time.Sleep(time.Second)
   }
   ```
   ```suggestion
                resp, err := svc.Greet(ctx, &greet.GreetRequest{Name: "Go 
Client"})
                if err != nil {
                        logger.Errorf("failed to greet: %v", err)
                }
                logger.Infof("Greet response: %s", resp.Greeting)
   
                cancel() // Cancel immediately after use
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to