Copilot commented on code in PR #96:
URL: 
https://github.com/apache/incubator-seata-go-samples/pull/96#discussion_r3408020844


##########
xa/bank_transfer/bank-a-service/main.go:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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"
+       "database/sql"
+       "errors"
+       "fmt"
+       "net/http"
+       "os"
+
+       "github.com/gin-gonic/gin"
+       "seata.apache.org/seata-go/pkg/client"
+       sql2 "seata.apache.org/seata-go/pkg/datasource/sql"
+       ginmiddleware "seata.apache.org/seata-go/pkg/integration/gin"
+)
+
+const defaultListenAddr = ":18081"
+
+var db *sql.DB
+
+type debitRequest struct {
+       AccountNo string `json:"account_no" binding:"required"`
+       Amount    int64  `json:"amount" binding:"required,gt=0"`
+}
+
+type account struct {
+       AccountNo string `json:"account_no"`
+       Balance   int64  `json:"balance"`
+}
+
+func main() {
+       client.InitPath(resolveSeataConfig())
+       db = openXADB()
+       defer db.Close()
+
+       r := gin.Default()
+       r.ContextWithFallback = true
+       r.GET("/accounts/:accountNo", accountHandler)
+       r.Use(ginmiddleware.TransactionMiddleware())
+       r.POST("/debit", debitHandler)
+
+       addr := getenv("BANK_A_ADDR", defaultListenAddr)
+       if err := r.Run(addr); err != nil {
+               panic(fmt.Sprintf("start bank-a-service failed: %v", err))
+       }
+}
+
+func debitHandler(c *gin.Context) {
+       var req debitRequest
+       if err := c.ShouldBindJSON(&req); err != nil {
+               c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+               return
+       }
+
+       if err := debit(c.Request.Context(), req.AccountNo, req.Amount); err != 
nil {
+               c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+               return
+       }
+       c.JSON(http.StatusOK, gin.H{"status": "debited"})
+}
+
+func accountHandler(c *gin.Context) {
+       acc, err := getAccount(c, c.Param("accountNo"))

Review Comment:
   `getAccount` expects a `context.Context`, but this handler passes 
`*gin.Context` which does not satisfy that interface and will not compile. Use 
the request context instead (also aligns with the README guidance about passing 
`c.Request.Context()` into SQL calls).



##########
xa/bank_transfer/transfer-service/main.go:
##########
@@ -0,0 +1,149 @@
+/*
+ * 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 (
+       "bytes"
+       "context"
+       "encoding/json"
+       "fmt"
+       "io"
+       "net/http"
+       "os"
+       "time"
+
+       "github.com/gin-gonic/gin"
+       "seata.apache.org/seata-go/pkg/client"
+       "seata.apache.org/seata-go/pkg/constant"
+       "seata.apache.org/seata-go/pkg/tm"
+)
+
+const defaultListenAddr = ":18080"
+
+var httpClient = &http.Client{Timeout: 10 * time.Second}
+
+type transferRequest struct {
+       FromAccountNo string `json:"from_account_no" binding:"required"`
+       ToAccountNo   string `json:"to_account_no" binding:"required"`
+       Amount        int64  `json:"amount" binding:"required,gt=0"`
+}
+
+func main() {
+       client.InitPath(resolveSeataConfig())
+
+       r := gin.Default()
+       r.POST("/transfer", func(c *gin.Context) {
+               var req transferRequest
+               if err := c.ShouldBindJSON(&req); err != nil {
+                       c.JSON(http.StatusBadRequest, gin.H{"error": 
err.Error()})
+                       return
+               }
+               executeTransfer(c, req)
+       })
+       r.POST("/transfer/success", func(c *gin.Context) {
+               executeTransfer(c, transferRequest{
+                       FromAccountNo: "A-1001",
+                       ToAccountNo:   "B-2001",
+                       Amount:        100,
+               })
+       })
+       r.POST("/transfer/fail", func(c *gin.Context) {
+               executeTransfer(c, transferRequest{
+                       FromAccountNo: "A-1001",
+                       ToAccountNo:   "B-FROZEN",
+                       Amount:        100,
+               })
+       })
+
+       addr := getenv("TRANSFER_ADDR", defaultListenAddr)
+       if err := r.Run(addr); err != nil {
+               panic(fmt.Sprintf("start transfer-service failed: %v", err))
+       }
+}
+
+func executeTransfer(c *gin.Context, req transferRequest) {
+       err := tm.WithGlobalTx(c.Request.Context(), &tm.GtxConfig{
+               Name:    "XABankTransfer",
+               Timeout: 30 * time.Second,
+       }, func(txCtx context.Context) error {
+               if err := postJSON(txCtx, getenv("BANK_A_URL", 
"http://127.0.0.1:18081";)+"/debit", map[string]any{
+                       "account_no": req.FromAccountNo,
+                       "amount":     req.Amount,
+               }); err != nil {
+                       return err
+               }
+
+               if err := postJSON(txCtx, getenv("BANK_B_URL", 
"http://127.0.0.1:18082";)+"/credit", map[string]any{
+                       "account_no": req.ToAccountNo,
+                       "amount":     req.Amount,
+               }); err != nil {
+                       return err
+               }
+               return nil
+       })
+       if err != nil {
+               c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+               return
+       }
+       c.JSON(http.StatusOK, gin.H{"status": "committed"})
+}
+
+func postJSON(ctx context.Context, url string, payload map[string]any) error {
+       body, err := json.Marshal(payload)
+       if err != nil {
+               return err
+       }
+
+       req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, 
bytes.NewReader(body))
+       if err != nil {
+               return err
+       }
+       req.Header.Set("Content-Type", "application/json")
+       req.Header.Set(constant.XidKey, tm.GetXID(ctx))
+
+       resp, err := httpClient.Do(req)
+       if err != nil {
+               return fmt.Errorf("post %s failed: %w", url, err)
+       }
+       defer resp.Body.Close()
+
+       respBody, _ := io.ReadAll(resp.Body)
+       if resp.StatusCode < http.StatusOK || resp.StatusCode >= 
http.StatusMultipleChoices {
+               return fmt.Errorf("post %s returned %s: %s", url, resp.Status, 
string(respBody))
+       }
+       return nil

Review Comment:
   The response body read error is ignored. If `io.ReadAll` fails (e.g., 
connection reset), the returned error message can be misleading and you lose 
the real failure cause.



##########
xa/bank_transfer/docker-compose.yml:
##########
@@ -0,0 +1,48 @@
+#
+# 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.
+#
+
+services:
+  seata-server:
+    image: seataio/seata-server:latest

Review Comment:
   Using `seataio/seata-server:latest` makes the sample non-reproducible and 
can break unexpectedly when the upstream image changes. Pin to a known working 
Seata server version (the repo already uses `1.6.1` in other samples).



##########
xa/bank_transfer/bank-b-service/main.go:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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"
+       "database/sql"
+       "errors"
+       "fmt"
+       "net/http"
+       "os"
+
+       "github.com/gin-gonic/gin"
+       "seata.apache.org/seata-go/pkg/client"
+       sql2 "seata.apache.org/seata-go/pkg/datasource/sql"
+       ginmiddleware "seata.apache.org/seata-go/pkg/integration/gin"
+)
+
+const defaultListenAddr = ":18082"
+
+var db *sql.DB
+
+type creditRequest struct {
+       AccountNo string `json:"account_no" binding:"required"`
+       Amount    int64  `json:"amount" binding:"required,gt=0"`
+}
+
+type account struct {
+       AccountNo string `json:"account_no"`
+       Balance   int64  `json:"balance"`
+       Frozen    bool   `json:"frozen"`
+}
+
+func main() {
+       client.InitPath(resolveSeataConfig())
+       db = openXADB()
+       defer db.Close()
+
+       r := gin.Default()
+       r.ContextWithFallback = true
+       r.GET("/accounts/:accountNo", accountHandler)
+       r.Use(ginmiddleware.TransactionMiddleware())
+       r.POST("/credit", creditHandler)
+
+       addr := getenv("BANK_B_ADDR", defaultListenAddr)
+       if err := r.Run(addr); err != nil {
+               panic(fmt.Sprintf("start bank-b-service failed: %v", err))
+       }
+}
+
+func creditHandler(c *gin.Context) {
+       var req creditRequest
+       if err := c.ShouldBindJSON(&req); err != nil {
+               c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+               return
+       }
+
+       if err := credit(c.Request.Context(), req.AccountNo, req.Amount); err 
!= nil {
+               c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+               return
+       }
+       c.JSON(http.StatusOK, gin.H{"status": "credited"})
+}
+
+func accountHandler(c *gin.Context) {
+       acc, err := getAccount(c, c.Param("accountNo"))

Review Comment:
   `getAccount` expects a `context.Context`, but this handler passes 
`*gin.Context` which does not satisfy that interface and will not compile. Use 
the request context instead.



-- 
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