Skip to content

Commit

Permalink
identity: improve error messages and wrapping (#901)
Browse files Browse the repository at this point in the history
  • Loading branch information
bnewbold authored Jan 7, 2025
2 parents 5e1b394 + 2d37f7c commit 67e2360
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 22 deletions.
7 changes: 4 additions & 3 deletions atproto/identity/base_directory.go
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
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
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
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
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

0 comments on commit 67e2360

Please sign in to comment.