Repository navigation
Fix image avatars never appearing on the watch - #105
Merged
Merged
Conversation
…e watch
A member with an uploaded avatar rendered as nothing at all on the watch: no
image, and not even the initials circle, on Currently Fronting, on Switch
Front, and on every tile. Members using an emoji or initials were fine, and the
same avatars were fine on the phone, which is what made it look like a layout
fault rather than a missing picture.
The server returns an avatar in its canonical stored form, a server-relative
path, and expects the client to resolve it. The phone has done that since the
CDN work, in a Coil interceptor that also honours `file_cdn_base`. The watch had
nothing equivalent and handed the raw path to both of the places it fetches
images, where it failed silently in two different ways: Coil cannot fetch a
string with no scheme or host, and `Request.Builder().url("/v1/...")` throws,
inside a `runCatching` that swallowed it and returned a null bitmap.
One resolver for the module, with the phone's rules verbatim so the two clients
cannot disagree about where a file lives, used in both places. The Compose side
gets it through an interceptor on the watch's own ImageLoader rather than at
each call site: seven screens draw avatars and none of them should have to know
about `file_cdn_base`. The tile pipeline builds its own OkHttp requests, so it
is handed the two bases directly.
`file_cdn_base` is fetched from the same unauthenticated config endpoint the
login screen already reads, and refreshed on each sync rather than once at
pairing, so an operator adding or dropping a CDN is picked up without a re-pair.
It is public configuration, so it sits in the plain settings store rather than
the encrypted credential one.
While here: an avatar that fails to load now falls back to the initials circle
instead of leaving an empty slot, so the next change in URL form degrades
visibly rather than silently.
The resolver's rules are pinned by test, including that a host which merely
shares a prefix with the instance is not the instance and must not be rewritten
onto the CDN. Verified on a watch against an instance serving relative avatar
URLs: the image now renders on both Currently Fronting and Switch Front.
The entry read as though image avatars were broken on the watch everywhere. They were not: whether the server returns a relative path or a full address depends on how the instance stores files. S3 with a public URL returns an absolute URL, which the watch coped with by accident, so an instance backed by separate file hosting never showed the bug. Serving files through the API is the default, so the reach is self-hosted instances rather than everyone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A member with an uploaded avatar rendered as nothing at all on the watch. Not the image, and not the initials circle either: an empty slot, on Currently Fronting, on Switch Front, and on every tile. Members using an emoji or initials were fine, and the same avatars were fine on the phone, which is what made it read as a layout fault rather than a missing picture.
Which instances this affected
Not all of them, and not the ones backed by separate file hosting, which is why this was never visible there.
resolve_avatar_urlreturns a different shape depending on how the instance stores files. S3 withs3_public_urlset returns an absolute URL. Everything else returns a path:/v1/files/{key}, signed or not. The watch coped with the first by accident, because there was nothing to work out, and failed on the second.storage_backenddefaults tofilesystem, so serving files through the API is the default configuration and self-hosted instances are the ones that saw this.Cause
Clients are expected to resolve the path form, and an instance may serve files from a different host entirely, which
file_cdn_basein the public config names. The phone has done this since the CDN work, in a Coil interceptor. The watch had nothing equivalent and passed the raw path to both places it fetches images, where it failed silently in two different ways:AsyncImagedrew nothing.Request.Builder().url("/v1/...")throws, inside arunCatchingthat swallowed it and handed back a null bitmap, so the tile drew its fallback.Neither logged anything a user or a maintainer would see.
Fix
One resolver for the wear module, with the phone's rules verbatim so the two clients cannot disagree about where a file lives, used in both places.
The Compose side gets it through an interceptor on the watch's own
ImageLoaderrather than at each call site: seven screens draw avatars and none of them should have to know aboutfile_cdn_base. The tile pipeline builds its own OkHttp requests and so never goes through Coil, so it is handed the two bases directly.file_cdn_basecomes from the same unauthenticated config endpoint the login screen already reads, refreshed on each sync rather than once at pairing, so an operator adding, changing or dropping a CDN is picked up without a re-pair. It is public configuration, so it lives in the plain settings store rather than the encrypted credential one.Note this also means an instance that later moves its files, or drops its separate host, keeps working on the watch instead of trading one silent failure for another.
While in here: an avatar that fails to load now falls back to the initials circle instead of leaving an empty slot. That is what made this bug so quiet, and it means the next change in URL form degrades visibly rather than invisibly.
Verification
Checked on a watch against an instance serving files through its own API, same emulator and same member either side. Before the fix, Currently Fronting showed the name and timer with an empty slot where the avatar belongs; after it, the image is there. Switch Front the same. The avatar shown is the uploaded image rather than the fallback, which would have been a letter on the member's colour.
Tests pin the resolver's rules: relative, bare relative, absolute-on-the-API-host, absolute-elsewhere, with and without a CDN, trailing slashes on either base, and nothing to resolve against. Including one that a host which merely shares a prefix with the instance is not the instance and must not be rewritten onto the CDN. I checked that one fails when the prefix check is weakened.
This unblocks the watch screenshots in the user docs, which were held because captures with an image-avatar fronter showed the empty slot. Those captures run against a stack that serves files itself, which is why they hit it.