Skip to content

dao: fix InitTx returning nil error on Begin failure#2720

Open
nankingjing wants to merge 1 commit into
coze-dev:mainfrom
nankingjing:fix/init-tx-nil-error
Open

dao: fix InitTx returning nil error on Begin failure#2720
nankingjing wants to merge 1 commit into
coze-dev:mainfrom
nankingjing:fix/init-tx-nil-error

Conversation

@nankingjing

Copy link
Copy Markdown

Bug

In InitTx(), when dao.DB.Begin() fails (rare, e.g. connection error), the function returns (nil, nil) instead of (nil, tx.Error) because it returns the named return err which is uninitialized (always nil).

Root cause

func (dao *KnowledgeDAO) InitTx() (tx *gorm.DB, err error) {
    tx = dao.DB.Begin()
    if tx.Error != nil {
        return nil, err   // err is nil here (named return, never assigned)
    }
    return
}

The named return err is never assigned a value in this function. When tx.Error is non-nil, line 88 returns (nil, nil) — the caller receives a nil error, proceeds to use the nil tx, and panics with a nil pointer dereference.

Fix

Return tx.Error instead of err on the error path:

        return nil, tx.Error

This is a pure Go language semantics bug — the named-error pattern works when err is assigned upstream, but it is not in this function. Exact same class as the CSV/user.go nil-error bugs fixed previously.

Verification

Verified by reading the source; no Go toolchain available on this machine. The bug is provable by static analysis: err is a named return of type error, default-initialized to nil, and never assigned in the function body.

File changed

backend/domain/knowledge/internal/dal/dao/knowledge.go line 88

@nankingjing

Copy link
Copy Markdown
Author

Review: fix/init-tx-nil-error (#2720)

Verdict: LGTM

Same class of bug as #2719: named-return-variable shadowing a real error. The function signature is func (dao *KnowledgeDAO) InitTx() (tx *gorm.DB, err error). When tx.Error != nil, the old code called return nil, err where err is the zero-value named return (nil). This caused InitTx to return (nil, nil) on Begin failure -- the caller gets a nil tx AND a nil error, which is the worst possible outcome.

The fix correctly returns nil, tx.Error, propagating the GORM Begin error to the caller.

This is a minimal, correct one-line fix. No test included but the logic is self-evident. Merge-ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant