this repo has no description
0
fork

Configure Feed

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

identity: refactor cache directory to prevent nil/nil condition (#479)

This is closely related to
https://github.com/bluesky-social/indigo/pull/458, which tried to
resolve an issue with DIDs. I was seeing very confusing behavior with
handles; was able to confirm `"", nil` getting returned internally on
real-world lookups.

I took a more aggressive approach and changed the return signatures of
both update helper methods to just return an "Entry", not as a pointer,
and no err. This makes the code flow much simpler.

I tacked on a special control-flow error message as defensive
programming after getting bit twice on this (darn self!). Probably isn't
necessary and smells a bit but :shrug:.

authored by

bnewbold and committed by
GitHub
a0cf12cf d8b9f66f

+23 -17
+23 -17
atproto/identity/cache_directory.go
··· 90 90 return false 91 91 } 92 92 93 - func (d *CacheDirectory) updateHandle(ctx context.Context, h syntax.Handle) (*HandleEntry, error) { 93 + func (d *CacheDirectory) updateHandle(ctx context.Context, h syntax.Handle) HandleEntry { 94 94 ident, err := d.Inner.LookupHandle(ctx, h) 95 95 if err != nil { 96 96 he := HandleEntry{ ··· 99 99 Err: err, 100 100 } 101 101 d.handleCache.Add(h, he) 102 - return &he, nil 102 + return he 103 103 } 104 104 105 105 entry := IdentityEntry{ ··· 115 115 116 116 d.identityCache.Add(ident.DID, entry) 117 117 d.handleCache.Add(ident.Handle, he) 118 - return &he, nil 118 + return he 119 119 } 120 120 121 121 func (d *CacheDirectory) ResolveHandle(ctx context.Context, h syntax.Handle) (syntax.DID, error) { ··· 145 145 } 146 146 } 147 147 148 - var did syntax.DID 149 148 // Update the Handle Entry from PLC and cache the result 150 - newEntry, err := d.updateHandle(ctx, h) 151 - if err == nil && newEntry != nil { 152 - did = newEntry.DID 153 - } 149 + newEntry := d.updateHandle(ctx, h) 150 + 154 151 // Cleanup the coalesce map and close the results channel 155 152 d.handleLookupChans.Delete(h.String()) 156 153 // Callers waiting will now get the result from the cache 157 154 close(res) 158 155 159 - return did, err 156 + if newEntry.Err != nil { 157 + return "", newEntry.Err 158 + } 159 + if newEntry.DID != "" { 160 + return newEntry.DID, nil 161 + } 162 + return "", fmt.Errorf("unexpected control-flow error") 160 163 } 161 164 162 - func (d *CacheDirectory) updateDID(ctx context.Context, did syntax.DID) (*IdentityEntry, error) { 165 + func (d *CacheDirectory) updateDID(ctx context.Context, did syntax.DID) IdentityEntry { 163 166 ident, err := d.Inner.LookupDID(ctx, did) 164 167 // persist the identity lookup error, instead of processing it immediately 165 168 entry := IdentityEntry{ ··· 181 184 if he != nil { 182 185 d.handleCache.Add(ident.Handle, *he) 183 186 } 184 - return &entry, err 187 + return entry 185 188 } 186 189 187 190 func (d *CacheDirectory) LookupDID(ctx context.Context, did syntax.DID) (*Identity, error) { ··· 211 214 } 212 215 } 213 216 214 - var doc *Identity 215 217 // Update the Identity Entry from PLC and cache the result 216 - newEntry, err := d.updateDID(ctx, did) 217 - if err == nil && newEntry != nil { 218 - doc = newEntry.Identity 219 - } 218 + newEntry := d.updateDID(ctx, did) 219 + 220 220 // Cleanup the coalesce map and close the results channel 221 221 d.didLookupChans.Delete(did.String()) 222 222 // Callers waiting will now get the result from the cache 223 223 close(res) 224 224 225 - return doc, err 225 + if newEntry.Err != nil { 226 + return nil, newEntry.Err 227 + } 228 + if newEntry.Identity != nil { 229 + return newEntry.Identity, nil 230 + } 231 + return nil, fmt.Errorf("unexpected control-flow error") 226 232 } 227 233 228 234 func (d *CacheDirectory) LookupHandle(ctx context.Context, h syntax.Handle) (*Identity, error) {