Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

identity: improve error messages and wrapping #901

Merged
merged 1 commit into from
Jan 7, 2025
Merged

identity: improve error messages and wrapping #901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions atproto/identity/base_directory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ func (d *BaseDirectory) LookupHandle(ctx context.Context, h syntax.Handle) (*Ide
ident := ParseIdentity(doc)
declared, err := ident.DeclaredHandle()
if err != nil {
return nil, err
return nil, fmt.Errorf("could not verify handle/DID match: %w", err)
}
if declared != h {
return nil, ErrHandleMismatch
return nil, fmt.Errorf("%w: %s != %s", ErrHandleMismatch, declared, h)
}
ident.Handle = declared

Expand All @@ -66,7 +66,7 @@ func (d *BaseDirectory) LookupDID(ctx context.Context, did syntax.DID) (*Identit
if errors.Is(err, ErrHandleNotDeclared) {
ident.Handle = syntax.HandleInvalid
} else if err != nil {
return nil, err
return nil, fmt.Errorf("could not parse handle from DID document: %w", err)
} else {
// if a handle was declared, resolve it
resolvedDID, err := d.ResolveHandle(ctx, declared)
Expand Down Expand Up @@ -99,5 +99,6 @@ func (d *BaseDirectory) Lookup(ctx context.Context, a syntax.AtIdentifier) (*Ide
}

func (d *BaseDirectory) Purge(ctx context.Context, a syntax.AtIdentifier) error {
// BaseDirectory itself does not implement caching
return nil
}
6 changes: 3 additions & 3 deletions atproto/identity/cache_directory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (d *CacheDirectory) updateHandle(ctx context.Context, h syntax.Handle) Hand

func (d *CacheDirectory) ResolveHandle(ctx context.Context, h syntax.Handle) (syntax.DID, error) {
if h.IsInvalidHandle() {
return "", fmt.Errorf("invalid handle")
return "", fmt.Errorf("can not resolve handle: %w", ErrInvalidHandle)
}
entry, ok := d.handleCache.Get(h)
if ok && !d.IsHandleStale(&entry) {
Expand Down Expand Up @@ -230,10 +230,10 @@ func (d *CacheDirectory) LookupHandleWithCacheState(ctx context.Context, h synta

declared, err := ident.DeclaredHandle()
if err != nil {
return nil, hit, err
return nil, hit, fmt.Errorf("could not verify handle/DID mapping: %w", err)
}
if declared != h {
return nil, hit, ErrHandleMismatch
return nil, hit, fmt.Errorf("%w: %s != %s", ErrHandleMismatch, declared, h)
}
return ident, hit, nil
}
Expand Down
3 changes: 3 additions & 0 deletions atproto/identity/directory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ var ErrDIDResolutionFailed = errors.New("DID resolution failed")
// Indicates that DID document did not include a public key with the specified ID
var ErrKeyNotDeclared = errors.New("DID document did not declare a relevant public key")

// Handle was invalid, in a situation where a valid handle is required.
var ErrInvalidHandle = errors.New("Invalid Handle")

var DefaultPLCURL = "https://plc.directory"

// Returns a reasonable Directory implementation for applications
Expand Down
8 changes: 4 additions & 4 deletions atproto/identity/handle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (d *BaseDirectory) ResolveHandleDNS(ctx context.Context, handle syntax.Hand
var dnsErr *net.DNSError
if errors.As(err, &dnsErr) {
if dnsErr.IsNotFound {
return "", ErrHandleNotFound
return "", fmt.Errorf("%w: %s", ErrHandleNotFound, handle)
}
}
if err != nil {
Expand Down Expand Up @@ -138,7 +138,7 @@ func (d *BaseDirectory) ResolveHandleWellKnown(ctx context.Context, handle synta
var dnsErr *net.DNSError
if errors.As(err, &dnsErr) {
if dnsErr.IsNotFound {
return "", fmt.Errorf("%w: DNS NXDOMAIN for %s", ErrHandleNotFound, handle)
return "", fmt.Errorf("%w: DNS NXDOMAIN for HTTP well-known resolution of %s", ErrHandleNotFound, handle)
}
}
return "", fmt.Errorf("%w: HTTP well-known request error: %w", ErrHandleResolutionFailed, err)
Expand All @@ -162,7 +162,7 @@ func (d *BaseDirectory) ResolveHandleWellKnown(ctx context.Context, handle synta
line := strings.TrimSpace(string(b))
outDid, err := syntax.ParseDID(line)
if err != nil {
return outDid, fmt.Errorf("bad well-known for %s: %w", handle, err)
return outDid, fmt.Errorf("%w: invalid DID in HTTP well-known for %s", ErrHandleResolutionFailed, handle)
}
return outDid, err
}
Expand All @@ -173,7 +173,7 @@ func (d *BaseDirectory) ResolveHandle(ctx context.Context, handle syntax.Handle)
var did syntax.DID

if handle.IsInvalidHandle() {
return "", fmt.Errorf("invalid handle")
return "", fmt.Errorf("can not resolve handle: %w", ErrInvalidHandle)
}

if !handle.AllowedTLD() {
Expand Down
24 changes: 12 additions & 12 deletions atproto/identity/redisdir/redis_directory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@ var _ identity.Directory = (*RedisDirectory)(nil)
func NewRedisDirectory(inner identity.Directory, redisURL string, hitTTL, errTTL, invalidHandleTTL time.Duration, lruSize int) (*RedisDirectory, error) {
opt, err := redis.ParseURL(redisURL)
if err != nil {
return nil, err
return nil, fmt.Errorf("could not configure redis identity cache: %w", err)
}
rdb := redis.NewClient(opt)
// check redis connection
_, err = rdb.Ping(context.TODO()).Result()
if err != nil {
return nil, err
return nil, fmt.Errorf("could not connect to redis identity cache: %w", err)
}
handleCache := cache.New(&cache.Options{
Redis: rdb,
Expand Down Expand Up @@ -117,7 +117,7 @@ func (d *RedisDirectory) updateHandle(ctx context.Context, h syntax.Handle) hand
})
if err != nil {
he.DID = nil
he.Err = fmt.Errorf("identity cache write: %w", err)
he.Err = fmt.Errorf("identity cache write failed: %w", err)
return he
}
return he
Expand All @@ -142,7 +142,7 @@ func (d *RedisDirectory) updateHandle(ctx context.Context, h syntax.Handle) hand
})
if err != nil {
he.DID = nil
he.Err = fmt.Errorf("identity cache write: %w", err)
he.Err = fmt.Errorf("identity cache write failed: %w", err)
return he
}
err = d.handleCache.Set(&cache.Item{
Expand All @@ -153,20 +153,20 @@ func (d *RedisDirectory) updateHandle(ctx context.Context, h syntax.Handle) hand
})
if err != nil {
he.DID = nil
he.Err = fmt.Errorf("identity cache write: %w", err)
he.Err = fmt.Errorf("identity cache write failed: %w", err)
return he
}
return he
}

func (d *RedisDirectory) ResolveHandle(ctx context.Context, h syntax.Handle) (syntax.DID, error) {
if h.IsInvalidHandle() {
return "", errors.New("invalid handle")
return "", fmt.Errorf("can not resolve handle: %w", identity.ErrInvalidHandle)
}
var entry handleEntry
err := d.handleCache.Get(ctx, redisDirPrefix+h.String(), &entry)
if err != nil && err != cache.ErrCacheMiss {
return "", fmt.Errorf("identity cache read: %w", err)
return "", fmt.Errorf("identity cache read failed: %w", err)
}
if err == nil && !d.isHandleStale(&entry) { // if no error...
handleCacheHits.Inc()
Expand All @@ -191,7 +191,7 @@ func (d *RedisDirectory) ResolveHandle(ctx context.Context, h syntax.Handle) (sy
// The result should now be in the cache
err := d.handleCache.Get(ctx, redisDirPrefix+h.String(), entry)
if err != nil && err != cache.ErrCacheMiss {
return "", fmt.Errorf("identity cache read: %w", err)
return "", fmt.Errorf("identity cache read failed: %w", err)
}
if err == nil && !d.isHandleStale(&entry) { // if no error...
if entry.Err != nil {
Expand Down Expand Up @@ -251,7 +251,7 @@ func (d *RedisDirectory) updateDID(ctx context.Context, did syntax.DID) identity
})
if err != nil {
entry.Identity = nil
entry.Err = fmt.Errorf("identity cache write: %v", err)
entry.Err = fmt.Errorf("identity cache write failed: %w", err)
return entry
}
if he != nil {
Expand All @@ -263,7 +263,7 @@ func (d *RedisDirectory) updateDID(ctx context.Context, did syntax.DID) identity
})
if err != nil {
entry.Identity = nil
entry.Err = fmt.Errorf("identity cache write: %v", err)
entry.Err = fmt.Errorf("identity cache write failed: %w", err)
return entry
}
}
Expand All @@ -279,7 +279,7 @@ func (d *RedisDirectory) LookupDIDWithCacheState(ctx context.Context, did syntax
var entry identityEntry
err := d.identityCache.Get(ctx, redisDirPrefix+did.String(), &entry)
if err != nil && err != cache.ErrCacheMiss {
return nil, false, fmt.Errorf("identity cache read: %v", err)
return nil, false, fmt.Errorf("identity cache read failed: %w", err)
}
if err == nil && !d.isIdentityStale(&entry) { // if no error...
identityCacheHits.Inc()
Expand All @@ -298,7 +298,7 @@ func (d *RedisDirectory) LookupDIDWithCacheState(ctx context.Context, did syntax
// The result should now be in the cache
err = d.identityCache.Get(ctx, redisDirPrefix+did.String(), &entry)
if err != nil && err != cache.ErrCacheMiss {
return nil, false, fmt.Errorf("identity cache read: %v", err)
return nil, false, fmt.Errorf("identity cache read failed: %w", err)
}
if err == nil && !d.isIdentityStale(&entry) { // if no error...
return entry.Identity, false, entry.Err
Expand Down
Loading