cloudwego / cloudwego/eino

bug: nil pointer dereference in flow/retriever/router when Router config is not provided

Open Beginner friendly
#954 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
13k
Forks
1.1k
Avg merge
4h 6m
Merged PRs (30d)
41

Description

## Bug Description

In `flow/retriever/router/router.go`, the `NewRetriever` function builds a default router function when `config.Router` is `nil`, but then accidentally stores `config.Router` (which is still `nil`) into the `routerRetriever` struct instead of the local `router` variable. This causes a **nil pointer dereference panic** when `Retrieve()` is called without a custom `Router` configured.

## Affected File

`flow/retriever/router/router.go`

## Root Cause

```go
func NewRetriever(ctx context.Context, config *Config) (retriever.Retriever, error) {
// ...
router := config.Router
if router == nil {
// correctly build a default router
var retrieverSet []string
for k := range config.Retrievers {
retrieverSet = append(retrieverSet, k)
}
router = func(ctx context.Context, query string) ([]string, error) {
return retrieverSet, nil
}
}
// ...
return &routerRetriever{
retrievers: config.Retrievers,
router: config.Router, // BUG: should be `router`, not `config.Router`
fusionFunc: fusion,
}, nil
}
```

When `config.Router` is `nil` (the common case where the user relies on the default all-retrievers router), the struct field `routerRetriever.router` is also `nil`. The `Retrieve()` method then calls `e.router(routeCtx, query)`, which panics with a nil pointer dereference.

## Steps to Reproduce

```go
retrieverA := /* some retriever.Retriever */
retrieverB := /* some retriever.Retriever */

// config.Router is intentionally omitted — expecting the default "all retrievers" behavior
rr, err := router.NewRetriever(ctx, &router.Config{
Retrievers: map[string]retriever.Retriever{
"a": retrieverA,
"b": retrieverB,
},
// Router not set, FusionFunc not set
})
if err != nil {
log.Fatal(err)
}

// PANICS: runtime error: invalid memory address or nil pointer dereference
docs, err := rr.Retrieve(ctx, "some query")
```

## Expected Behavior

`NewRetriever` should store the resolved (possibly default) `router` variable into the struct, not `config.Router`. When `config.Router` is `nil`, all registered retrievers should be queried (the intended default behavior).

## Fix

Change line in `NewRetriever`:
```go
// Before (buggy)
return &routerRetriever{
retrievers: config.Retrievers,
router: config.Router,
fusionFunc: fusion,
}, nil

// After (fixed)
return &routerRetriever{
retrievers: config.Retrievers,
router: router,
fusionFunc: fusion,
}, nil
```

## Impact

Any caller that omits `Config.Router` (relying on the documented default behavior of querying all retrievers) will encounter a nil pointer dereference panic at runtime instead of the intended behavior.

Contributor guide

Open the contributing guide

Research direction

Read flow/retriever/router/router.go, starting with NewRetriever and then Retrieve. Reproduce the issue with Config.Router omitted and registered retrievers, then verify the default configuration no longer panics and queries the configured retrievers.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.