269 lines
14 KiB
Markdown
269 lines
14 KiB
Markdown
# SDK issues FA Droid
|
||
|
||
Bugs whose root cause is in the **FA SDK** (`go-fa-api`, at
|
||
`/var/home/soxx/git/go-fa-api`, `replace`d in `go.mod`) not in this app.
|
||
|
||
The SDK is a **separate module, maintained and fixed by a separate AI**
|
||
that does not see this app's code or conversation context. This file is the
|
||
handoff: every entry is a self-contained bug report meant to be copy-pasted
|
||
to that AI as-is.
|
||
|
||
App-side bugs (frontend / go-service) live in `ISSUES.md`; fixed issues in
|
||
`SOLVED.md`. Features with UI but no backend are in `STUBS.md`; Wails/Android
|
||
stack traps in `PITFALLS.md`.
|
||
|
||
**Status legend:** `[ ]` open · `[~]` diagnosed handoff brief drafted ·
|
||
`[x]` fixed in the SDK (then moved to `SOLVED.md`)
|
||
**Numbering:** issue numbers are shared across `ISSUES.md`, `SOLVED.md` and
|
||
this file draw the next free number from the same sequence.
|
||
|
||
---
|
||
|
||
## SDK handoff brief required fields
|
||
|
||
Every entry MUST carry a handoff brief detailed enough to be copy-pasted to
|
||
the SDK AI as a standalone bug report. Do not write "sdk issue?" and stop —
|
||
that is not actionable for someone without this repo open.
|
||
|
||
A handoff brief must answer all seven of:
|
||
|
||
1. **SDK entry point** the exact exported function involved
|
||
(e.g. `Client.Fav(ctx, SubmissionID)` in `actions.go`). Name the file.
|
||
2. **How the app calls it** which `internal/services/*.go` wrapper
|
||
invokes it, and with what arguments.
|
||
3. **Observed behaviour** what the SDK call returns: an error (quote it
|
||
verbatim), a wrong value, or a silent no-op.
|
||
4. **Expected behaviour** what FurAffinity should do server-side and
|
||
what the SDK should return.
|
||
5. **Reproduction** concrete inputs (submission ID, username) and the
|
||
steps that trigger it.
|
||
6. **Suspected layer** HTML parsing (FA changed its markup?), request
|
||
shape (wrong form fields / missing CSRF / wrong URL), auth/cookies, or
|
||
rate-limiting. Point at the likely file: parsers are `*_parser.go`,
|
||
form posts are in `actions.go` / `*_post.go` / `*_send.go`.
|
||
7. **What is NOT the SDK** explicitly rule out the app layer so the SDK
|
||
AI does not chase a frontend bug. State what you verified.
|
||
|
||
If you cannot yet fill all seven fields, the issue stays `[~]` and the brief
|
||
lists exactly which diagnostics are still needed never hand over a
|
||
half-brief as if it were complete.
|
||
|
||
---
|
||
|
||
## Issues
|
||
|
||
### #4 [~] Audit the FA SDK for bugs
|
||
- **Where:** `go-fa-api` (sibling module, `replace`d in `go.mod`).
|
||
- **Task:** Standing umbrella review the SDK surface used by
|
||
`internal/services/` for incorrect parsing, missing endpoints, or wrong
|
||
request shapes.
|
||
- **Cause tag:** `sdk`
|
||
- **Progress:** The first audit pass produced #1, #5, #9, #10 and #14 **all
|
||
now fixed and verified** (#1/#5/#9/#10 in app v0.0.7, #14 in v0.0.10; see
|
||
`SOLVED.md`). #15 was then fixed too (SDK gave `WithResolvedAvatars` a
|
||
count limit; app v0.0.14). #17 (priority rate limiting) was implemented too
|
||
— verified in app v0.0.16. **No open SDK issues currently.** Keep this
|
||
entry as the umbrella for future audit passes; file each concrete finding
|
||
as its own numbered issue.
|
||
|
||
_No open SDK issues. Fixed ones are recorded in `SOLVED.md`._
|
||
|
||
---
|
||
|
||
## Add new SDK issues below
|
||
|
||
<!-- #N title (N = next free number, shared with ISSUES.md)
|
||
- Where:
|
||
- Symptom:
|
||
- Expected:
|
||
- Cause tag: sdk
|
||
- SDK handoff brief:
|
||
1. Entry point:
|
||
2. How the app calls it:
|
||
3. Observed behaviour:
|
||
4. Expected behaviour:
|
||
5. Reproduction:
|
||
6. Suspected layer:
|
||
7. What is NOT the SDK:
|
||
-->
|
||
|
||
### #18 [x] Rate limiter: upgrade from 2-level to N-level priority
|
||
|
||
**Implemented in the SDK update** `options.go` now exposes `WithPriority`
|
||
and a multi-level `Priority` type; `WithPrioritizedRateLimiting` honours it.
|
||
FA Droid follow-up (assign tiers to reads/writes/preload/crawl) is tracked as
|
||
app-side work, not an SDK issue.
|
||
|
||
|
||
- **Component:** `go-fa-api` `ratelimit.go`, `options.go`.
|
||
- **Today:** the limiter has exactly two priorities. `WithBackgroundPriority(ctx)`
|
||
marks a request background; everything else is foreground. Background
|
||
requests wait until no foreground request is queued. Enabled via
|
||
`WithPrioritizedRateLimiting(true)`.
|
||
- **Need:** a consumer (FA Droid) wants *three+* tiers, not two:
|
||
1. user-interactive (the page on screen, user write actions),
|
||
2. speculative neighbor preload (likely-next submissions),
|
||
3. bulk background crawl (inbox/watchlist warming).
|
||
With only two levels, neighbor preload must either compete with the live
|
||
page (tier 1) or interleave with the bulk crawl (tier 2) neither is right.
|
||
- **Proposed API:** keep `WithBackgroundPriority` working (maps to the lowest
|
||
level) and add `WithPriority(ctx, Priority)` where `Priority` is an ordered
|
||
enum, e.g. `PriorityInteractive`, `PriorityNormal` (default),
|
||
`PriorityLow`, `PriorityBackground`. The limiter serves waiting goroutines
|
||
highest-priority-first; the token emission rate is unchanged.
|
||
- **Compatibility:** default (no marker) must remain `PriorityNormal`;
|
||
`WithBackgroundPriority` must remain equivalent to `PriorityBackground`.
|
||
- **Why it matters to the app:** FA Droid will then assign tier 1 to
|
||
current-page reads + the write-action queue worker, tier `PriorityLow` to
|
||
neighbor preload, and `PriorityBackground` to the inbox crawler.
|
||
|
||
### #21 [x] GetSubmission doesn't expose the viewer's favorite state
|
||
|
||
- **Where:** `go-fa-api` `submission.go` (`Submission` struct + `parseSubmission`).
|
||
- **Symptom:** A submission the logged-in user has favorited shows the
|
||
favorite heart as *empty* on the app's submission detail page. There is no
|
||
way to know a submission's favorite state from a `GetSubmission` result.
|
||
- **Expected:** `GetSubmission` should report whether the authenticated viewer
|
||
has favorited the submission, so the UI can render the correct heart state.
|
||
- **Cause tag:** `sdk`
|
||
- **SDK handoff brief:**
|
||
1. **Entry point:** `Client.GetSubmission(ctx, SubmissionID)` in
|
||
`submission.go`, which builds the result via `parseSubmission(id, doc)`.
|
||
The returned `Submission` struct (`submission.go:17-38`) has fields
|
||
ID, Title, Author, PostedAt, Rating, Category, Type, Species, Gender,
|
||
Description, DescriptionText, Tags, FileURL, ThumbURL, Width, Height,
|
||
Stats, Folders, Prev, Next and **nothing indicating favorite state**.
|
||
2. **How the app calls it:** `SubmissionService.getCached` in
|
||
`internal/services/submission.go` calls `GetSubmission`, then
|
||
`dto.FromSubmission` (`internal/dto/types.go`) copies the struct to the
|
||
wire DTO. The frontend (`SubmissionView.svelte`) does
|
||
`favorited = !!sub.favorited`.
|
||
3. **Observed behaviour:** `GetSubmission` returns a `*Submission` with no
|
||
favorite-state field. It is not a wrong value or an error the datum is
|
||
simply absent from the SDK's public type.
|
||
4. **Expected behaviour:** When `/view/{id}/` is fetched with valid `a`/`b`
|
||
cookies, FA renders either a `+Fav` (`/fav/{id}/...`) or a `−Fav`
|
||
(`/unfav/{id}/...`) anchor exactly one, matching the viewer's current
|
||
state. The SDK should surface this on `Submission`, e.g. a new
|
||
`Favorited bool` field (final name your call), set true when the page
|
||
shows the `/unfav/` link. On an anonymous (no-cookie) fetch neither link
|
||
is present → `Favorited` false, which is correct.
|
||
5. **Reproduction:** With cookies set, favorite submission X on FA, then
|
||
call `GetSubmission(ctx, X)` the result cannot express that X is
|
||
favorited.
|
||
6. **Suspected layer:** HTML parsing `parseSubmission`. The SDK *already*
|
||
has the exact parser: `findFavLinks(doc, subID) (favURL, unfavURL string)`
|
||
in `actions.go` (used by `toggleFavorite` for its idempotency check).
|
||
`unfavURL != ""` means "currently favorited." `parseSubmission` just needs
|
||
to run that check and set the new field. No new scraping logic required.
|
||
7. **What is NOT the SDK:** The app side is verified ready and correct.
|
||
`SubmissionView.svelte` already reads `sub.favorited` and types it
|
||
(`favorited?: boolean`); `getCached` correctly busts and re-fetches the
|
||
submission cache after a fav write (via `WriteService` / the action
|
||
queue). The value never reaches the app only because the SDK `Submission`
|
||
struct has no field to carry it `dto.FromSubmission` has nothing to map.
|
||
- **Related (please also check):** the same gap likely exists for *watch*
|
||
state `findWatchLinks` exists in `actions.go`, but the `User` struct may
|
||
not expose whether the viewer watches that user. Worth fixing in the same
|
||
pass.
|
||
- **When it lands (FA Droid follow-up):** add `Favorited bool` to
|
||
`dto.Submission` (`json:"favorited"`), map it in `dto.FromSubmission`, and
|
||
regenerate bindings. The frontend already consumes `sub.favorited`, so no UI
|
||
change is needed.
|
||
- **Status:** done SDK update added `Submission.Favorited`; app wired it in
|
||
v0.0.22.
|
||
|
||
### #23 [ ] SubmissionInbox yields only the first page (~72 items)
|
||
|
||
- **Where:** `go-fa-api` `inbox.go` (`SubmissionInbox` + `parseSubmissionInboxPage`).
|
||
- **Symptom:** The new-submission inbox shows only ~72 items even when the
|
||
account has thousands pending. A user with ~4718 inbox submissions sees one
|
||
page.
|
||
- **Expected:** `SubmissionInbox` should walk every cursor page until FA stops
|
||
rendering a "Next 72" link.
|
||
- **Cause tag:** `sdk`
|
||
- **SDK handoff brief:**
|
||
1. **Entry point:** `Client.SubmissionInbox(ctx, ListOptions)` in `inbox.go`,
|
||
whose iterator follows `parseSubmissionInboxPage`'s returned `nextURL`.
|
||
2. **How the app calls it:** `InboxService.StreamSubmissions`
|
||
(`internal/services/inbox.go`) runs one goroutine that does
|
||
`for sub, err := range client.SubmissionInbox(ctx, fa.ListOptions{})`
|
||
— `ListOptions{}` means `MaxPages: 0` (unbounded), so the app does not
|
||
cap the crawl.
|
||
3. **Observed behaviour:** the `range` completes after ~72 items the
|
||
iterator yields one page then ends. Verified on-device: the app's crawl
|
||
channel (fed one item per `yield`) closes after exactly one ~72-item
|
||
chunk, so `StreamSubmissions` reports `HasMore: false` immediately after
|
||
page 1.
|
||
4. **Expected behaviour:** FA's `/msg/submissions/` paginates via a
|
||
"Next 72" link encoding a from-id cursor; the iterator should follow it
|
||
across all ~66 pages for a 4718-item inbox.
|
||
5. **Reproduction:** logged-in client with a large submission inbox;
|
||
`count := 0; for range client.SubmissionInbox(ctx, ListOptions{}) { count++ }`
|
||
yields ~72, not the true total.
|
||
6. **Suspected layer:** HTML parsing `parseSubmissionInboxPage`. Its next-
|
||
cursor selector is `div.messagecenter-navigation a.button.more`; if FA
|
||
changed that markup the parser returns `nextURL == ""` and the iterator
|
||
stops after page 1. Check the selector against current `/msg/submissions/`
|
||
HTML (and the cursor-URL construction).
|
||
7. **What is NOT the SDK:** app side verified. `StreamSubmissions` ranges the
|
||
iterator fully on a single goroutine and streams everything it yields; it
|
||
passes `ListOptions{}` (no `MaxPages` cap). On-device logging shows the
|
||
crawl ends after one chunk the iterator simply stops yielding.
|
||
|
||
### #24 [x] Request logger drops `context.Context` (breaks trace propagation)
|
||
|
||
**Fixed in the SDK** `transport.go`'s `logRequest` now emits its record via
|
||
`logger.InfoContext(req.Context(), "fa.request", …)` instead of `logger.Info`,
|
||
so a context-aware `slog.Handler` recovers the caller's active span and the
|
||
HTTP span nests under the RPC span. A regression test,
|
||
`TestTransport_LogRequest_PropagatesRequestContext` in `transport_test.go`,
|
||
installs a context-capturing handler and asserts a sentinel value threaded
|
||
through `req.Context()` reaches the slog record guarding against a silent
|
||
revert to `Info`.
|
||
|
||
|
||
- **Where:** `go-fa-api` the HTTP transport's request logging (`transport.go`,
|
||
the `logRequest` helper / wherever `slog` records an outgoing request).
|
||
- **Type:** Enhancement, not a bug the SDK works correctly; this is a
|
||
one-line change the app needs for distributed tracing.
|
||
- **Symptom:** The app (FA Droid, WI-10) added OpenTelemetry spans. An app RPC
|
||
opens an `rpc` span and threads its `context.Context` into the SDK call. The
|
||
SDK's per-request `slog` line is currently emitted with `logger.Info(...)`,
|
||
which carries **no context** so the app's `slog` handler cannot recover the
|
||
active span, and each HTTP span becomes an unparented root instead of a child
|
||
of the RPC span.
|
||
- **Expected:** the request log record should carry the request's context so a
|
||
context-aware `slog.Handler` can read the active span from it.
|
||
- **The fix (one line):** change the request-logging call from
|
||
`logger.Info("fa.request", …)` to
|
||
`logger.InfoContext(req.Context(), "fa.request", …)` (use the `*http.Request`'s
|
||
own `Context()`). `slog` already supports `InfoContext`; no API change, no new
|
||
dependency.
|
||
- **Cause tag:** `sdk`
|
||
- **SDK handoff brief:**
|
||
1. **SDK entry point:** the HTTP transport `RoundTrip` / request path in
|
||
`transport.go` specifically the `slog` call that logs each outgoing FA
|
||
request (`logRequest`, or inline). All SDK client methods route through it.
|
||
2. **How the app calls it:** every `internal/services/*.go` wrapper calls a
|
||
`Client.*` method with a `context.Context` that now carries an OTel span;
|
||
that ctx reaches the `*http.Request` (`req.Context()`).
|
||
3. **Observed behaviour:** the request `slog` record is created without a
|
||
context, so `Handler.Handle` receives `context.Background()`.
|
||
4. **Expected behaviour:** the record carries `req.Context()`, so a
|
||
context-aware handler can extract the active span.
|
||
5. **Reproduction:** with WI-10's `diag` slog handler installed, every HTTP
|
||
span has `parentSpanId == ""` even when the call was made inside an RPC
|
||
span. After the `InfoContext` change, the HTTP span nests under the RPC
|
||
span on the same trace id.
|
||
6. **Suspected layer:** request shape / logging purely the logging call,
|
||
no parsing or auth involved.
|
||
7. **What is NOT the SDK behaviour-wise:** no functional SDK behaviour
|
||
changes request execution, parsing, retries, rate-limiting are all
|
||
untouched. This only changes which `slog` method is used so context flows.
|
||
- **Note:** WI-10 applied this change to the *local* `replace`d working copy of
|
||
`go-fa-api` so FA Droid v0.0.33 has working rpc→http span linkage. It is
|
||
**not committed in the SDK repo**. Until it is upstreamed, a clean SDK
|
||
checkout will degrade gracefully HTTP spans become valid but unparented
|
||
roots (no crash, no data loss, only the rpc↔http link is lost).
|