encore-go-code-review

// 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) {
    // ...
}

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

// 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) {
    // ...
}

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

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

// 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 {
    // ...
}

// 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)
        },
    },
)

// 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()
}

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