encore-go-code-review

Compare original and translation side by side

🇺🇸

Original

English

🇨🇳

Translation

Chinese

Encore Go Code Review

Encore Go代码审查

Instructions

说明

When reviewing Encore Go code, check for these common issues:

在审查Encore Go代码时，检查以下常见问题：

Critical Issues

严重问题

1. Infrastructure Inside Functions

1. 函数内声明基础设施

// WRONG: Infrastructure declared inside function
func setup() {
    db := sqldb.NewDatabase("mydb", sqldb.DatabaseConfig{...})
    topic := pubsub.NewTopic[*Event]("events", pubsub.TopicConfig{...})
}

// CORRECT: Package level declaration
var db = sqldb.NewDatabase("mydb", sqldb.DatabaseConfig{
    Migrations: "./migrations",
})

var topic = pubsub.NewTopic[*Event]("events", pubsub.TopicConfig{
    DeliveryGuarantee: pubsub.AtLeastOnce,
})

// 错误：在函数内声明基础设施
func setup() {
    db := sqldb.NewDatabase("mydb", sqldb.DatabaseConfig{...})
    topic := pubsub.NewTopic[*Event]("events", pubsub.TopicConfig{...})
}

// 正确：包级声明
var db = sqldb.NewDatabase("mydb", sqldb.DatabaseConfig{
    Migrations: "./migrations",
})

var topic = pubsub.NewTopic[*Event]("events", pubsub.TopicConfig{
    DeliveryGuarantee: pubsub.AtLeastOnce,
})

2. Missing Context Parameter

2. 缺少Context参数

// WRONG: Missing context
//encore:api public method=GET path=/users/:id
func GetUser(params *GetUserParams) (*User, error) {
    // ...
}

// CORRECT: Context as first parameter
//encore:api public method=GET path=/users/:id
func GetUser(ctx context.Context, params *GetUserParams) (*User, error) {
    // ...
}

// 错误：缺少context
//encore:api public method=GET path=/users/:id
func GetUser(params *GetUserParams) (*User, error) {
    // ...
}

// 正确：将Context作为第一个参数
//encore:api public method=GET path=/users/:id
func GetUser(ctx context.Context, params *GetUserParams) (*User, error) {
    // ...
}

3. SQL Injection Risk

3. SQL注入风险

// WRONG: String interpolation
query := fmt.Sprintf("SELECT * FROM users WHERE email = '%s'", email)
rows, err := db.Query(ctx, query)

// CORRECT: Parameterized query
rows, err := sqldb.Query[User](ctx, db, `
    SELECT * FROM users WHERE email = $1
`, email)

// 错误：字符串拼接
query := fmt.Sprintf("SELECT * FROM users WHERE email = '%s'", email)
rows, err := db.Query(ctx, query)

// 正确：参数化查询
rows, err := sqldb.Query[User](ctx, db, `
    SELECT * FROM users WHERE email = $1
`, email)

4. Wrong Return Types

4. 返回类型错误

// WRONG: Returning non-pointer struct
//encore:api public method=GET path=/users/:id
func GetUser(ctx context.Context, params *GetUserParams) (User, error) {
    // ...
}

// CORRECT: Return pointer to struct
//encore:api public method=GET path=/users/:id
func GetUser(ctx context.Context, params *GetUserParams) (*User, error) {
    // ...
}

// 错误：返回非指针结构体
//encore:api public method=GET path=/users/:id
func GetUser(ctx context.Context, params *GetUserParams) (User, error) {
    // ...
}

// 正确：返回结构体指针
//encore:api public method=GET path=/users/:id
func GetUser(ctx context.Context, params *GetUserParams) (*User, error) {
    // ...
}

5. Ignoring Errors

5. 忽略错误

// WRONG: Ignoring error
user, _ := sqldb.QueryRow[User](ctx, db, query, id)

// CORRECT: Handle error
user, err := sqldb.QueryRow[User](ctx, db, query, id)
if err != nil {
    return nil, err
}

// 错误：忽略错误
user, _ := sqldb.QueryRow[User](ctx, db, query, id)

// 正确：处理错误
user, err := sqldb.QueryRow[User](ctx, db, query, id)
if err != nil {
    return nil, err
}

Warning Issues

警告问题

6. Not Checking for ErrNoRows

6. 未检查ErrNoRows

// RISKY: Returns nil without proper error
func getUser(ctx context.Context, id string) (*User, error) {
    user, err := sqldb.QueryRow[User](ctx, db, `
        SELECT * FROM users WHERE id = $1
    `, id)
    if err != nil {
        return nil, err  // ErrNoRows returns generic error
    }
    return user, nil
}

// BETTER: Check for not found specifically
import "errors"

func getUser(ctx context.Context, id string) (*User, error) {
    user, err := sqldb.QueryRow[User](ctx, db, `
        SELECT * FROM users WHERE id = $1
    `, id)
    if errors.Is(err, sqldb.ErrNoRows) {
        return nil, &errs.Error{
            Code:    errs.NotFound,
            Message: "user not found",
        }
    }
    if err != nil {
        return nil, err
    }
    return user, nil
}

// 风险：未返回合适的错误就返回nil
func getUser(ctx context.Context, id string) (*User, error) {
    user, err := sqldb.QueryRow[User](ctx, db, `
        SELECT * FROM users WHERE id = $1
    `, id)
    if err != nil {
        return nil, err  // ErrNoRows会返回通用错误
    }
    return user, nil
}

// 更佳：专门检查未找到的情况
import "errors"

func getUser(ctx context.Context, id string) (*User, error) {
    user, err := sqldb.QueryRow[User](ctx, db, `
        SELECT * FROM users WHERE id = $1
    `, id)
    if errors.Is(err, sqldb.ErrNoRows) {
        return nil, &errs.Error{
            Code:    errs.NotFound,
            Message: "用户未找到",
        }
    }
    if err != nil {
        return nil, err
    }
    return user, nil
}

7. Public Internal Endpoints

7. 公开的内部端点

// CHECK: Should this cron endpoint be public?
//encore:api public method=POST path=/internal/cleanup
func CleanupJob(ctx context.Context) error {
    // ...
}

// BETTER: Use private for internal endpoints
//encore:api private
func CleanupJob(ctx context.Context) error {
    // ...
}

// 检查：这个定时任务端点应该公开吗？
//encore:api public method=POST path=/internal/cleanup
func CleanupJob(ctx context.Context) error {
    // ...
}

// 更佳：内部端点使用private
//encore:api private
func CleanupJob(ctx context.Context) error {
    // ...
}

8. Non-Idempotent Subscription Handlers

8. 非幂等的订阅处理器

// RISKY: Not idempotent (pubsub has at-least-once delivery)
var _ = pubsub.NewSubscription(OrderCreated, "process-order",
    pubsub.SubscriptionConfig[*OrderCreatedEvent]{
        Handler: func(ctx context.Context, event *OrderCreatedEvent) error {
            return chargeCustomer(ctx, event.OrderID)  // Could charge twice!
        },
    },
)

// SAFER: Check before processing
var _ = pubsub.NewSubscription(OrderCreated, "process-order",
    pubsub.SubscriptionConfig[*OrderCreatedEvent]{
        Handler: func(ctx context.Context, event *OrderCreatedEvent) error {
            order, err := getOrder(ctx, event.OrderID)
            if err != nil {
                return err
            }
            if order.Status != "pending" {
                return nil  // Already processed
            }
            return chargeCustomer(ctx, event.OrderID)
        },
    },
)

// 风险：非幂等（pubsub至少投递一次）
var _ = pubsub.NewSubscription(OrderCreated, "process-order",
    pubsub.SubscriptionConfig[*OrderCreatedEvent]{
        Handler: func(ctx context.Context, event *OrderCreatedEvent) error {
            return chargeCustomer(ctx, event.OrderID)  // 可能重复扣费！
        },
    },
)

// 更安全：处理前先检查
var _ = pubsub.NewSubscription(OrderCreated, "process-order",
    pubsub.SubscriptionConfig[*OrderCreatedEvent]{
        Handler: func(ctx context.Context, event *OrderCreatedEvent) error {
            order, err := getOrder(ctx, event.OrderID)
            if err != nil {
                return err
            }
            if order.Status != "pending" {
                return nil  // 已处理过
            }
            return chargeCustomer(ctx, event.OrderID)
        },
    },
)

9. Not Closing Query Rows

9. 未关闭查询结果行

// WRONG: Rows not closed
func listUsers(ctx context.Context) ([]*User, error) {
    rows, err := sqldb.Query[User](ctx, db, `SELECT * FROM users`)
    if err != nil {
        return nil, err
    }
    // Missing: defer rows.Close()
    
    var users []*User
    for rows.Next() {
        users = append(users, rows.Value())
    }
    return users, nil
}

// CORRECT: Always close rows
func listUsers(ctx context.Context) ([]*User, error) {
    rows, err := sqldb.Query[User](ctx, db, `SELECT * FROM users`)
    if err != nil {
        return nil, err
    }
    defer rows.Close()
    
    var users []*User
    for rows.Next() {
        users = append(users, rows.Value())
    }
    return users, rows.Err()
}

// 错误：未关闭结果行
func listUsers(ctx context.Context) ([]*User, error) {
    rows, err := sqldb.Query[User](ctx, db, `SELECT * FROM users`)
    if err != nil {
        return nil, err
    }
    // 缺失：defer rows.Close()
    
    var users []*User
    for rows.Next() {
        users = append(users, rows.Value())
    }
    return users, nil
}

// 正确：始终关闭结果行
func listUsers(ctx context.Context) ([]*User, error) {
    rows, err := sqldb.Query[User](ctx, db, `SELECT * FROM users`)
    if err != nil {
        return nil, err
    }
    defer rows.Close()
    
    var users []*User
    for rows.Next() {
        users = append(users, rows.Value())
    }
    return users, rows.Err()
}

Review Checklist

审查检查清单

All infrastructure at package level
All API endpoints have
```
context.Context
```
as first parameter
SQL uses parameterized queries (
```
$1
```
,
```
$2
```
, etc.)
Response types are pointers
Errors are handled, not ignored
```
sqldb.ErrNoRows
```
checked where appropriate
Internal endpoints use
```
private
```
not
```
public
```
Subscription handlers are idempotent
Query rows are closed with
```
defer rows.Close()
```
Migrations follow naming convention (
```
1_name.up.sql
```
)

所有基础设施均为包级声明
所有API端点都将
```
context.Context
```
作为第一个参数
SQL使用参数化查询（
```
$1
```
、
```
$2
```
等）
响应类型为指针
错误已处理，未被忽略
在适当的地方检查
```
sqldb.ErrNoRows
```
内部端点使用
```
private
```
而非
```
public
```
订阅处理器是幂等的
使用
```
defer rows.Close()
```
关闭查询结果行
迁移遵循命名规范（
```
1_name.up.sql
```
）

Output Format

输出格式

When reviewing, report issues as:

[CRITICAL] [file:line] Description of issue
[WARNING] [file:line] Description of concern  
[GOOD] Notable good practice observed

审查时，按以下格式报告问题：

[严重] [文件:行号] 问题描述
[警告] [文件:行号] 注意事项描述  
[良好] 发现值得肯定的最佳实践