anthropics / anthropics/anthropic-sdk-go

bedrock.WithLoadDefaultConfig panics on ordinary AWS config failures (unknown profile, no SSO cache), so a CLI answers a login prompt with a stack trace

Đang mở
#416 1 bình luận 1 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
Go
Star
1.2k
Fork
213
Merge trung bình
1 ngày 12 giờ
Pull request đã merge (30 ngày)
11

Mô tả

### Summary

`bedrock.WithLoadDefaultConfig` panics when `config.LoadDefaultConfig` returns an error:

```go
func WithLoadDefaultConfig(ctx context.Context, optFns ...func(*config.LoadOptions) error) option.RequestOption {
cfg, err := config.LoadDefaultConfig(ctx, optFns...)
if err != nil {
panic(err)
}
return WithConfig(cfg)
}
```

`bedrock/bedrock.go:186-192` (v1.55.1)

Every failure this converts into a panic is an ordinary, recoverable, user-caused condition rather than a programmer error: a profile name that does not exist, a missing or malformed `~/.aws/config`, an SSO cache that was never populated. For a CLI those are the common case on first run, so the tool answers "you have not logged in yet" with a stack trace, and the user cannot tell a misconfiguration from a crash.

### Versions

- `github.com/anthropics/anthropic-sdk-go v1.55.1`
- `github.com/aws/aws-sdk-go-v2/config v1.32.34`
- go1.25.6, darwin/arm64 (not platform-specific)

### Reproduction

No AWS setup required beyond a profile name that does not exist:

```go
package main

import (
"context"
"fmt"

"github.com/anthropics/anthropic-sdk-go"
"github.com/anthropics/anthropic-sdk-go/bedrock"
)

func main() {
defer func() {
if r := recover(); r != nil {
fmt.Printf("PANICKED: %v\n", r)
}
}()
_ = anthropic.NewClient(bedrock.WithLoadDefaultConfig(context.Background()))
fmt.Println("no panic")
}
```

```console
$ AWS_PROFILE=definitely-not-a-real-profile AWS_REGION=us-east-1 go run .
PANICKED: failed to get shared config profile, definitely-not-a-real-profile
```

Without the `recover`, that is a stack trace and a non-zero exit.

### Expected

The failure is surfaced as an error the caller can present, not a panic.

### Why it is awkward to fix, and two options

`option.RequestOption` has no error channel, which is presumably why the panic is there. Two ways out, either of which would work for us:

1. **An erroring variant**, leaving the current function untouched for callers who genuinely want to fail fast:

```go
func WithLoadDefaultConfigErr(ctx context.Context, optFns ...func(*config.LoadOptions) error) (option.RequestOption, error)
```

2. **Defer the failure to the first request**: capture the error in the returned option and have the middleware return it, so a caller that never issues a request never sees it and one that does gets a normal `error`.

### Workaround

Call `config.LoadDefaultConfig` directly, keep the error, and pass the config to `bedrock.WithConfig` — deferring the failure to the first request with a message naming the likely fix. That is what we do in [alibaba/open-code-review](https://github.com/alibaba/open-code-review), and it is the only reason an expired SSO session there produces a sentence instead of a stack trace.

Note for anyone landing here from the same direction: `bedrock.WithConfig` needs its own care on SSO profiles — see #414 and #415.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.