this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

identity: refactor error hierarchy and error handling (#478)

Root motivation here was that a bunch of remote errors were trickling up
as full errors when doing identity resolution (via DID), while in
reality they should result in "invalid.handle".

I didn't want to just force everything in to ErrHandleNotFound, so I
created some new error types (and renamed a couple existing types), and
ensured that we were using errors.Is() for matching. This way downstream
code can distinguish between "not found" and "error during resolution"
when calling the lower-level methods, if they care (eg, a debugging tool
or interface would care). Also wrapped the errors instead of just
returning, so additional context couple be tacked on the message.

There are still some situations where the root cause might be a local
error (like local wifi) as opposed to a remote error (server is broken),
and we can't distinguish those, at least for now.

Note: I checked and recent versions of golang does explicitly allow
multiple `%w` in a single `fmt.Errorf()`

authored by

bnewbold and committed by
GitHub
8f423633 d3006f89

+77 -55
+20 -11
atproto/identity/base_directory.go
··· 2 2 3 3 import ( 4 4 "context" 5 + "errors" 5 6 "fmt" 6 7 "net" 7 8 "net/http" ··· 48 49 return nil, err 49 50 } 50 51 if declared != h { 51 - return nil, fmt.Errorf("handle does not match that declared in DID document") 52 + return nil, ErrHandleMismatch 52 53 } 53 54 ident.Handle = declared 54 55 55 - // optimistic caching of public key 56 + // optimistic pre-parsing of public key 56 57 pk, err := ident.PublicKey() 57 58 if nil == err { 58 59 ident.ParsedPublicKey = pk ··· 67 68 } 68 69 ident := ParseIdentity(doc) 69 70 declared, err := ident.DeclaredHandle() 70 - if err != nil { 71 + if errors.Is(err, ErrHandleNotDeclared) { 72 + ident.Handle = syntax.HandleInvalid 73 + } else if err != nil { 71 74 return nil, err 72 - } 73 - resolvedDID, err := d.ResolveHandle(ctx, declared) 74 - if err != nil && err != ErrHandleNotFound { 75 - return nil, err 76 - } else if ErrHandleNotFound == err || resolvedDID != did { 77 - ident.Handle = syntax.HandleInvalid 78 75 } else { 79 - ident.Handle = declared 76 + // if a handle was declared, resolve it 77 + resolvedDID, err := d.ResolveHandle(ctx, declared) 78 + if err != nil { 79 + if errors.Is(err, ErrHandleNotFound) || errors.Is(err, ErrHandleResolutionFailed) { 80 + ident.Handle = syntax.HandleInvalid 81 + } else { 82 + return nil, err 83 + } 84 + } else if resolvedDID != did { 85 + ident.Handle = syntax.HandleInvalid 86 + } else { 87 + ident.Handle = declared 88 + } 80 89 } 81 90 82 - // optimistic caching of public key 91 + // optimistic pre-parsing of public key 83 92 pk, err := ident.PublicKey() 84 93 if nil == err { 85 94 ident.ParsedPublicKey = pk
+1 -1
atproto/identity/cache_directory.go
··· 247 247 return nil, err 248 248 } 249 249 if declared != h { 250 - return nil, fmt.Errorf("handle does not match that declared in DID document") 250 + return nil, ErrHandleMismatch 251 251 } 252 252 return ident, nil 253 253 }
+9 -9
atproto/identity/did.go
··· 79 79 var dnsErr *net.DNSError 80 80 if errors.As(err, &dnsErr) { 81 81 if dnsErr.IsNotFound { 82 - return nil, ErrDIDNotFound 82 + return nil, fmt.Errorf("%w: DNS NXDOMAIN", ErrDIDNotFound) 83 83 } 84 84 } 85 85 if err != nil { 86 - return nil, fmt.Errorf("failed HTTP fetch of did:web well-known document: %w", err) 86 + return nil, fmt.Errorf("%w: did:web HTTP well-known fetch: %w", ErrDIDResolutionFailed, err) 87 87 } 88 88 if resp.StatusCode == http.StatusNotFound { 89 - return nil, ErrDIDNotFound 89 + return nil, fmt.Errorf("%w: did:web HTTP status 404", ErrDIDNotFound) 90 90 } 91 91 if resp.StatusCode != http.StatusOK { 92 - return nil, fmt.Errorf("failed did:web well-known fetch, HTTP status: %d", resp.StatusCode) 92 + return nil, fmt.Errorf("%w: did:web HTTP status %d", ErrDIDResolutionFailed, resp.StatusCode) 93 93 } 94 94 95 95 var doc DIDDocument 96 96 if err := json.NewDecoder(resp.Body).Decode(&doc); err != nil { 97 - return nil, fmt.Errorf("failed parse of did:web document JSON: %w", err) 97 + return nil, fmt.Errorf("%w: JSON DID document parse: %w", ErrDIDResolutionFailed, err) 98 98 } 99 99 return &doc, nil 100 100 } ··· 117 117 118 118 resp, err := http.Get(plcURL + "/" + did.String()) 119 119 if err != nil { 120 - return nil, fmt.Errorf("failed did:plc directory resolution: %w", err) 120 + return nil, fmt.Errorf("%w: PLC directory lookup: %w", ErrDIDResolutionFailed, err) 121 121 } 122 122 if resp.StatusCode == http.StatusNotFound { 123 - return nil, ErrDIDNotFound 123 + return nil, fmt.Errorf("%w: PLC directory 404", ErrDIDNotFound) 124 124 } 125 125 if resp.StatusCode != http.StatusOK { 126 - return nil, fmt.Errorf("failed did:plc resolution, HTTP status: %d", resp.StatusCode) 126 + return nil, fmt.Errorf("%w: PLC directory status %d", ErrDIDResolutionFailed, resp.StatusCode) 127 127 } 128 128 129 129 var doc DIDDocument 130 130 if err := json.NewDecoder(resp.Body).Decode(&doc); err != nil { 131 - return nil, fmt.Errorf("failed parse of did:plc document JSON: %w", err) 131 + return nil, fmt.Errorf("%w: JSON DID document parse: %w", ErrDIDResolutionFailed, err) 132 132 } 133 133 return &doc, nil 134 134 }
+1 -1
atproto/identity/did_test.go
··· 68 68 assert.Equal([]string{}, id.AlsoKnownAs) 69 69 pk, err := id.PublicKey() 70 70 assert.Error(err) 71 - assert.Equal(ErrKeyNotFound, err) 71 + assert.ErrorIs(err, ErrKeyNotDeclared) 72 72 assert.Nil(pk) 73 73 assert.Equal("", id.PDSEndpoint()) 74 74 hdl, err := id.DeclaredHandle()
+18 -15
atproto/identity/handle.go
··· 20 20 parts := strings.SplitN(s, "=", 2) 21 21 did, err := syntax.ParseDID(parts[1]) 22 22 if err != nil { 23 - return "", fmt.Errorf("invalid DID in handle DNS record: %w", err) 23 + return "", fmt.Errorf("%w: invalid DID in handle DNS record: %w", ErrHandleResolutionFailed, err) 24 24 } 25 25 return did, nil 26 26 } ··· 39 39 } 40 40 } 41 41 if err != nil { 42 - return "", fmt.Errorf("handle DNS resolution failed: %w", err) 42 + return "", fmt.Errorf("%w: %w", ErrHandleResolutionFailed, err) 43 43 } 44 44 return parseTXTResp(res) 45 45 } ··· 56 56 } 57 57 } 58 58 if err != nil { 59 - return "", fmt.Errorf("handle DNS resolution failed: %w", err) 59 + return "", fmt.Errorf("%w: DNS error: %w", ErrHandleResolutionFailed, err) 60 60 } 61 61 if len(resNS) == 0 { 62 62 return "", ErrHandleNotFound ··· 84 84 } 85 85 } 86 86 if err != nil { 87 - return "", fmt.Errorf("handle DNS resolution failed: %w", err) 87 + return "", fmt.Errorf("%w: DNS resolution failed: %w", ErrHandleResolutionFailed, err) 88 88 } 89 89 return parseTXTResp(res) 90 90 } ··· 113 113 } 114 114 } 115 115 if err != nil { 116 - retErr = err 116 + retErr = fmt.Errorf("%w: %w", ErrHandleResolutionFailed, err) 117 117 continue 118 118 } 119 119 ret, err := parseTXTResp(res) ··· 129 129 func (d *BaseDirectory) ResolveHandleWellKnown(ctx context.Context, handle syntax.Handle) (syntax.DID, error) { 130 130 req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("https://%s/.well-known/atproto-did", handle), nil) 131 131 if err != nil { 132 - return "", err 132 + return "", fmt.Errorf("constructing HTTP request for handle resolution: %w", err) 133 133 } 134 134 135 135 resp, err := d.HTTPClient.Do(req) ··· 138 138 var dnsErr *net.DNSError 139 139 if errors.As(err, &dnsErr) { 140 140 if dnsErr.IsNotFound { 141 - return "", ErrHandleNotFound 141 + return "", fmt.Errorf("%w: DNS NXDOMAIN for %s", ErrHandleNotFound, handle) 142 142 } 143 143 } 144 - return "", fmt.Errorf("failed to resolve handle (%s) through HTTP well-known route: %s", handle, err) 144 + return "", fmt.Errorf("%w: HTTP well-known request error: %w", ErrHandleResolutionFailed, err) 145 + } 146 + if resp.StatusCode == http.StatusNotFound { 147 + return "", fmt.Errorf("%w: HTTP 404 for %s", ErrHandleNotFound, handle) 145 148 } 146 149 if resp.StatusCode != http.StatusOK { 147 - return "", fmt.Errorf("failed to resolve handle (%s) through HTTP well-known route: status=%d", handle, resp.StatusCode) 150 + return "", fmt.Errorf("%w: HTTP well-known status %d for %s", ErrHandleResolutionFailed, resp.StatusCode, handle) 148 151 } 149 152 150 153 if resp.ContentLength > 2048 { 151 - return "", fmt.Errorf("HTTP well-known route returned too much data during handle resolution") 154 + return "", fmt.Errorf("%w: HTTP well-known body too large for %s", ErrHandleResolutionFailed, handle) 152 155 } 153 156 154 157 b, err := io.ReadAll(io.LimitReader(resp.Body, 2048)) 155 158 if err != nil { 156 - return "", fmt.Errorf("HTTP well-known response fail to read: %w", err) 159 + return "", fmt.Errorf("%w: HTTP well-known body read for %s: %w", ErrHandleResolutionFailed, handle, err) 157 160 } 158 161 line := strings.TrimSpace(string(b)) 159 162 return syntax.ParseDID(line) ··· 181 184 triedAuthoritative := false 182 185 triedFallback := false 183 186 did, dnsErr = d.ResolveHandleDNS(ctx, handle) 184 - if dnsErr == ErrHandleNotFound && d.TryAuthoritativeDNS { 187 + if errors.Is(dnsErr, ErrHandleNotFound) && d.TryAuthoritativeDNS { 185 188 slog.Info("attempting authoritative handle DNS resolution", "handle", handle) 186 189 triedAuthoritative = true 187 190 // try harder with authoritative lookup 188 191 did, dnsErr = d.ResolveHandleDNSAuthoritative(ctx, handle) 189 192 } 190 - if dnsErr == ErrHandleNotFound && len(d.FallbackDNSServers) > 0 { 193 + if errors.Is(dnsErr, ErrHandleNotFound) && len(d.FallbackDNSServers) > 0 { 191 194 slog.Info("attempting fallback DNS resolution", "handle", handle) 192 195 triedFallback = true 193 196 // try harder with fallback lookup ··· 209 212 } 210 213 211 214 // return the most specific/helpful error 212 - if dnsErr != ErrHandleNotFound { 215 + if !errors.Is(dnsErr, ErrHandleNotFound) { 213 216 return "", dnsErr 214 217 } 215 - if httpErr != ErrHandleNotFound { 218 + if !errors.Is(httpErr, ErrHandleNotFound) { 216 219 return "", httpErr 217 220 } 218 221 return "", dnsErr
+15 -6
atproto/identity/identity.go
··· 34 34 Purge(ctx context.Context, i syntax.AtIdentifier) error 35 35 } 36 36 37 - // Indicates that resolution process completed successfully, but handle does not exist. 37 + // Indicates that handle resolution failed. A wrapped error may provide more context. This is only returned when looking up a handle, not when looking up a DID. 38 + var ErrHandleResolutionFailed = errors.New("handle resolution failed") 39 + 40 + // Indicates that resolution process completed successfully, but handle does not exist. This is only returned when looking up a handle, not when looking up a DID. 38 41 var ErrHandleNotFound = errors.New("handle not found") 39 42 40 - // Indicates that handle and DID resolved, but handle points to a DID with a different handle. This is only returned when looking up a handle, not when looking up a DID. 41 - var ErrHandleNotValid = errors.New("handle resolves to DID with different handle") 43 + // Indicates that resolution process completed successfully, handle mapped to a different DID. This is only returned when looking up a handle, not when looking up a DID. 44 + var ErrHandleMismatch = errors.New("handle/DID mismatch") 45 + 46 + // Indicates that DID document did not include any handle ("alsoKnownAs"). This is only returned when looking up a handle, not when looking up a DID. 47 + var ErrHandleNotDeclared = errors.New("DID document did not declare a handle") 42 48 43 49 // Handle top-level domain (TLD) is one of the special "Reserved" suffixes, and not allowed for atproto use 44 50 var ErrHandleReservedTLD = errors.New("handle top-level domain is disallowed") ··· 46 52 // Indicates that resolution process completed successfully, but the DID does not exist. 47 53 var ErrDIDNotFound = errors.New("DID not found") 48 54 49 - var ErrKeyNotFound = errors.New("identity has no public repo signing key") 55 + // Indicates that DID resolution process failed. A wrapped error may provide more context. 56 + var ErrDIDResolutionFailed = errors.New("DID resolution failed") 57 + 58 + var ErrKeyNotDeclared = errors.New("identity has no public repo signing key") 50 59 51 60 var DefaultPLCURL = "https://plc.directory" 52 61 ··· 160 169 } 161 170 k, ok := i.Keys["atproto"] 162 171 if !ok { 163 - return nil, ErrKeyNotFound 172 + return nil, ErrKeyNotDeclared 164 173 } 165 174 switch k.Type { 166 175 case "Multikey": ··· 215 224 return syntax.ParseHandle(u[5:]) 216 225 } 217 226 } 218 - return "", fmt.Errorf("DID document contains no atproto handle") 227 + return "", ErrHandleNotDeclared 219 228 }
+9 -8
atproto/identity/live_test.go
··· 4 4 "context" 5 5 "log/slog" 6 6 "net/http" 7 + "strings" 7 8 "sync" 8 9 "testing" 9 10 "time" ··· 21 22 22 23 handle := syntax.Handle("atproto.com") 23 24 did := syntax.DID("did:plc:ewvi7nxzyoun6zhxrhs64oiz") 24 - pds := "https://bsky.social" 25 + pdsSuffix := "host.bsky.network" 25 26 26 27 resp, err := d.LookupHandle(ctx, handle) 27 28 assert.NoError(err) 28 29 assert.Equal(handle, resp.Handle) 29 30 assert.Equal(did, resp.DID) 30 - assert.Equal(pds, resp.PDSEndpoint()) 31 + assert.True(strings.HasSuffix(resp.PDSEndpoint(), pdsSuffix)) 31 32 dh, err := resp.DeclaredHandle() 32 33 assert.NoError(err) 33 34 assert.Equal(handle, dh) ··· 39 40 assert.NoError(err) 40 41 assert.Equal(handle, resp.Handle) 41 42 assert.Equal(did, resp.DID) 42 - assert.Equal(pds, resp.PDSEndpoint()) 43 + assert.True(strings.HasSuffix(resp.PDSEndpoint(), pdsSuffix)) 43 44 44 45 _, err = d.LookupHandle(ctx, syntax.Handle("fake-dummy-no-resolve.atproto.com")) 45 - assert.Equal(ErrHandleNotFound, err) 46 + assert.ErrorIs(err, ErrHandleNotFound) 46 47 47 48 _, err = d.LookupDID(ctx, syntax.DID("did:web:fake-dummy-no-resolve.atproto.com")) 48 - assert.Equal(ErrDIDNotFound, err) 49 + assert.ErrorIs(err, ErrDIDNotFound) 49 50 50 51 _, err = d.LookupDID(ctx, syntax.DID("did:plc:fake-dummy-no-resolve.atproto.com")) 51 - assert.Equal(ErrDIDNotFound, err) 52 + assert.ErrorIs(err, ErrDIDNotFound) 52 53 53 54 _, err = d.LookupHandle(ctx, syntax.HandleInvalid) 54 55 assert.Error(err) ··· 129 130 // valid DNS server 130 131 _, err := dir.LookupHandle(ctx, handle) 131 132 assert.Error(err) 132 - assert.Equal(ErrHandleNotFound, err) 133 + assert.ErrorIs(err, ErrHandleNotFound) 133 134 134 135 // invalid DNS server syntax 135 136 dir.FallbackDNSServers = []string{"_"} 136 137 _, err = dir.LookupHandle(ctx, handle) 137 138 assert.Error(err) 138 - assert.NotEqual(ErrHandleNotFound, err) 139 + assert.ErrorIs(err, ErrHandleResolutionFailed) 139 140 }
+4 -4
atproto/identity/mock_directory_test.go
··· 29 29 30 30 // first, empty directory 31 31 _, err = c.LookupHandle(ctx, syntax.Handle("handle.example.com")) 32 - assert.Equal(ErrHandleNotFound, err) 32 + assert.ErrorIs(err, ErrHandleNotFound) 33 33 _, err = c.LookupDID(ctx, syntax.DID("did:plc:abc123")) 34 - assert.Equal(ErrDIDNotFound, err) 34 + assert.ErrorIs(err, ErrDIDNotFound) 35 35 36 36 c.Insert(id1) 37 37 c.Insert(id2) ··· 49 49 assert.True(out.Handle.IsInvalidHandle()) 50 50 51 51 _, err = c.LookupHandle(ctx, syntax.HandleInvalid) 52 - assert.Equal(ErrHandleNotFound, err) 52 + assert.ErrorIs(err, ErrHandleNotFound) 53 53 out, err = c.LookupDID(ctx, syntax.DID("did:plc:abc999")) 54 - assert.Equal(ErrDIDNotFound, err) 54 + assert.ErrorIs(err, ErrDIDNotFound) 55 55 }