diff --git a/core/http/endpoints/addons/apps.go b/core/http/endpoints/addons/apps.go index 26c31f0be3..a22577cac6 100644 --- a/core/http/endpoints/addons/apps.go +++ b/core/http/endpoints/addons/apps.go @@ -28,7 +28,7 @@ var appID = regexp.MustCompile(`^[a-z][a-z0-9-]{0,63}$`) // @Success 200 {object} apps.List "Apps visible to the caller" // @Failure 401 {object} AddonErrorResponse "Named authentication required" // @Failure 403 {object} AddonErrorResponse "Feature or owner policy denied the request" -// @Failure 503 {object} AddonErrorResponse "Addons or the owner are unavailable, or the owner did not answer in time and this frontend has no earlier list to show (type owner_busy or owner_unavailable)" +// @Failure 503 {object} AddonErrorResponse "Addons or the owner are unavailable, or the owner did not answer in time (type owner_busy or owner_unavailable)" // @Router /api/addons/apps [get] func ListApps(g *gateway.Gateway, db *gorm.DB, opts ...Option) echo.HandlerFunc { s := resolve(opts) diff --git a/core/http/endpoints/addons/apps_test.go b/core/http/endpoints/addons/apps_test.go index 9b335c0bc6..cdef6e64ac 100644 --- a/core/http/endpoints/addons/apps_test.go +++ b/core/http/endpoints/addons/apps_test.go @@ -419,7 +419,7 @@ var _ = Describe("apps routes outcomes and documentation", Label("apps"), func() return m } full := apps.App{ID: "a", Name: "a", Credentialed: true, ContextFiles: []contract.ContextFile{{Scope: "user", Path: "Memory/index.md"}}, UserSettings: []contract.UserSetting{{Key: "save-mode", Values: []string{"ask", "never"}, Default: "ask", Role: "memory.save-mode"}}, Network: &apps.NetworkView{Kind: contract.NetworkKindEgressCheck, Title: "N", Presets: []contract.NetworkPreset{{ID: "off", Title: "Closed"}}}, Title: "t", Titles: map[string]string{"en": "t"}, Description: "d", Descriptions: map[string]string{"en": "d"}, Language: "it", LocalizedTitle: "t", LocalizedDescription: "d", Icon: "i", Version: "1", InstalledAt: "2026-10-30T09:15:00Z", Generation: 1, Enabled: true, State: "running", Update: "update-pending", Operation: &apps.Operation{Kind: "upgrade", Phase: "downloading", Since: "2026-11-04T17:18:00Z"}, MainPage: "/m", HasUI: true, LaunchMode: "always", Tools: []string{"x"}, ToolCount: 1, Shadows: []string{"bash"}, ToolDefinitions: []contract.ToolDefinition{{Name: "x", Description: "d", Kind: "read", InputSchema: json.RawMessage(`{"type":"object"}`), UIOnly: true, Approval: contract.ApprovalAlways, Shadows: "bash", Execution: &contract.ExecutionDescriptor{Class: "commands", Title: "T", Note: "n", Skills: &contract.ExecutionSkills{ProvidesRuntimeFor: []string{"documents"}}}}}, Skills: []contract.SkillDefinition{{Name: "xlsx", Shadows: "xlsx", Slot: &contract.SlotDefinition{Kind: contract.SlotKindDocuments}}}, SkillShadows: []string{"xlsx"}, UI: &apps.UIView{Entry: "/e", CSS: []string{"/c"}, SDK: contract.VersionRange{Min: 1, Max: 1}, Actions: []apps.ChatActionView{{ID: "a", Label: "A", Labels: map[string]string{"en": "A"}, Icon: "star", PackagedIcon: true, Placement: "composer", Tool: "t", Component: "C"}}, Slots: []apps.SlotView{{ID: "s", Slot: "files.preview", Component: "C", Extensions: []string{"docx"}}}, Renderers: []contract.Renderer{{Kind: "approval-card", Tool: "t", Component: "C"}}, Pages: []string{"p"}, Navigation: []string{"p"}, HomeCommands: []apps.HomeCommandView{{ID: "h", Page: "p", Label: "H", Icon: "star"}}, ModelActions: []contract.ModelAction{{Capability: "FLAG_IMAGE", Page: "p"}}, ChatActions: []string{"a"}, Controls: []string{"network-policy"}}, Admin: &apps.AdminView{CanStart: true, CanStop: true, ManagementURL: "/x", HostOperations: &hostops.RequestSummary{Required: []string{"r"}}}} - for name, v := range map[string]any{"apps.App": full, "apps.AdminView": *full.Admin, "apps.UIView": *full.UI, "apps.ChatActionView": full.UI.Actions[0], "apps.SlotView": full.UI.Slots[0], "contract.Renderer": full.UI.Renderers[0], "contract.VersionRange": full.UI.SDK, "contract.ToolDefinition": full.ToolDefinitions[0], "contract.ExecutionDescriptor": *full.ToolDefinitions[0].Execution, "contract.ExecutionSkills": *full.ToolDefinitions[0].Execution.Skills, "contract.SkillDefinition": full.Skills[0], "apps.List": apps.List{Apps: []apps.App{full}, Stale: true, StaleReason: "owner_busy", StaleSince: "2026-11-04T17:18:30Z"}, "apps.Operation": *full.Operation} { + for name, v := range map[string]any{"apps.App": full, "apps.AdminView": *full.Admin, "apps.UIView": *full.UI, "apps.ChatActionView": full.UI.Actions[0], "apps.SlotView": full.UI.Slots[0], "contract.Renderer": full.UI.Renderers[0], "contract.VersionRange": full.UI.SDK, "contract.ToolDefinition": full.ToolDefinitions[0], "contract.ExecutionDescriptor": *full.ToolDefinitions[0].Execution, "contract.ExecutionSkills": *full.ToolDefinitions[0].Execution.Skills, "contract.SkillDefinition": full.Skills[0], "apps.List": apps.List{Apps: []apps.App{full}}, "apps.Operation": *full.Operation} { def, ok := doc.Definitions[name] Expect(ok).To(BeTrue(), name) shape := emitted(v) diff --git a/core/http/react-ui/e2e/addons-busy-owner.spec.js b/core/http/react-ui/e2e/addons-busy-owner.spec.js index 344f7acd2c..5acf2d5a60 100644 --- a/core/http/react-ui/e2e/addons-busy-owner.spec.js +++ b/core/http/react-ui/e2e/addons-busy-owner.spec.js @@ -3,7 +3,7 @@ import { ADMIN, app, installedAddon, sessionFor, mockAddons, fail } from './addo // The owner is busy with an upgrade. The routes are stand-ins // (addons-fixtures.js); the owner's own behaviour is covered by the Go specs -// "reads during a long upgrade" and "apps list while the owner is busy". +// "reads during a long upgrade" and "apps list while the owner is busy". The server fails closed (a typed 503 on a timeout, never an older list); the page keeps its own last list.. const shots = process.env.BUSY_SHOTS const theme = process.env.BUSY_THEME @@ -58,17 +58,20 @@ test.describe('Add-ons: the owner is busy', () => { await expect(page.getByText('No add-ons yet')).toHaveCount(0) }) - test('a stale list from the server keeps the rows, names what is upgrading and shows the phase of each row', async ({ page }) => { + test('a timeout after a list with upgrades in flight keeps the rows, names what is upgrading and shows the phase of each row', async ({ page }) => { await signIn(page) + let busy = false await mockAddons(page, { apps: [notes(), sandbox(), tables()], installed: [installedAddon({ id: 'notes' })], - appsList: () => body({ - apps: [notes({ operation: upgrading('downloading') }), sandbox({ operation: upgrading('starting') }), tables()], - stale: true, staleReason: 'owner_busy', staleSince: '2026-11-04T17:18:30Z', - }), + appsList: () => (busy + ? fail(503, 'owner_busy', { message: 'busy', code: 503 }) + : body({ apps: [notes({ operation: upgrading('downloading') }), sandbox({ operation: upgrading('starting') }), tables()] })), }) await page.goto('/app/addons') + await expect(tile(page, 'notes').getByTestId('operation-chip')).toBeVisible() + busy = true + await page.reload() const banner = page.getByTestId('addons-stale') await expect(banner).toBeVisible() await expect(banner).toContainText('The add-on server is busy (upgrading Notes, Sandbox)') @@ -122,10 +125,14 @@ test.describe('Add-ons: the owner is busy', () => { await expect(page.getByTestId('addons-load-error')).toHaveCount(0) }) - test('a stale copy of an empty list is not the empty state either', async ({ page }) => { + test('a remembered empty list shown during a timeout is not the empty state either', async ({ page }) => { await signIn(page) - await mockAddons(page, { apps: [], appsList: () => body({ apps: [], stale: true, staleReason: 'owner_busy' }) }) + let busy = false + await mockAddons(page, { apps: [], appsList: () => (busy ? fail(503, 'owner_busy', { message: 'busy', code: 503 }) : null) }) await page.goto('/app/addons') + await expect(page.getByText('No add-ons yet').first()).toBeVisible() + busy = true + await page.reload() await expect(page.getByTestId('addons-load-error')).toBeVisible() await expect(page.getByText('No add-ons yet')).toHaveCount(0) }) diff --git a/core/http/react-ui/src/components/addons/AppsView.jsx b/core/http/react-ui/src/components/addons/AppsView.jsx index fa04c87fcd..b27b46a82d 100644 --- a/core/http/react-ui/src/components/addons/AppsView.jsx +++ b/core/http/react-ui/src/components/addons/AppsView.jsx @@ -47,7 +47,7 @@ const browserStore = () => { try { return globalThis.sessionStorage || null } ca export default function AppsView({ api, adapter, onSummary, grants }) { const { t, i18n } = useTranslation('addons') - const [storedState, setState] = useState({ status: 'loading', apps: [], error: null, stale: false, staleReason: '' }) + const [storedState, setState] = useState({ status: 'loading', apps: [], error: null, stale: false }) const [reload, setReload] = useState(0) const reloadRef = useRef(() => {}) const { hasFeature, isAdmin, user, sessionEpoch } = useAuth() @@ -59,7 +59,7 @@ export default function AppsView({ api, adapter, onSummary, grants }) { const actor = JSON.stringify([user?.id, sessionEpoch, isAdmin, canList]) const scope = useRef(actor) scope.current = actor - const state = storedState.actor === actor ? storedState : { status: 'loading', apps: [], error: null, stale: false, staleReason: '' } + const state = storedState.actor === actor ? storedState : { status: 'loading', apps: [], error: null, stale: false } const ingest = registry?.ingest // Poll with a timer chain: one request at a time, aborted on unmount. A @@ -80,16 +80,12 @@ export default function AppsView({ api, adapter, onSummary, grants }) { try { const read = await api.apps.read(ctl.signal) const apps = read.apps - // The server answered with its last copy: the owner did not. Ask - // again with the same backoff as for a failed read. - failures = read.stale ? failures + 1 : 0 + failures = 0 if (alive && current()) { - setState({ actor, status: 'ready', apps, error: null, stale: read.stale, staleReason: read.staleReason }) - if (!read.stale) { - ingest?.(apps) - const store = browserStore() - if (store) saveLastKnown(store, actor, apps) - } + setState({ actor, status: 'ready', apps, error: null, stale: false }) + ingest?.(apps) + const store = browserStore() + if (store) saveLastKnown(store, actor, apps) } } catch (error) { if (!alive || !current() || (error && error.name === 'AbortError')) return @@ -101,7 +97,7 @@ export default function AppsView({ api, adapter, onSummary, grants }) { const remembered = store ? loadLastKnown(store, actor) : null setState(s => { const kept = s.actor === actor && s.status === 'ready' ? s.apps : remembered - return { actor, status: kept ? 'ready' : 'error', apps: kept || [], error, stale: true, staleReason: '' } + return { actor, status: kept ? 'ready' : 'error', apps: kept || [], error, stale: true } }) } finally { inflight = false @@ -320,7 +316,7 @@ export default function AppsView({ api, adapter, onSummary, grants }) { title={t('apps.loadError.title')} testId="addons-load-error" nothingChanged={false} - onRetry={() => { setState({ status: 'loading', apps: [], error: null, stale: false, staleReason: '' }); setReload(n => n + 1) }} + onRetry={() => { setState({ status: 'loading', apps: [], error: null, stale: false }); setReload(n => n + 1) }} > {[ describeError(loadError, t), diff --git a/core/http/react-ui/src/components/addons/addonState.js b/core/http/react-ui/src/components/addons/addonState.js index 4eeb4ea3b0..ada5111777 100644 --- a/core/http/react-ui/src/components/addons/addonState.js +++ b/core/http/react-ui/src/components/addons/addonState.js @@ -319,13 +319,11 @@ export function upgradingTitles(apps) { } // Why the list on screen may be out of date: 'busy' (the owner did not answer -// in time or the server sent its last copy), 'unreachable' or '' when it is -// current. -export function staleKind({ error, stale, staleReason }) { +// in time), 'unreachable' or '' when it is current. +export function staleKind({ error, stale }) { if (!stale) return '' if (error && error.kind === 'busy') return 'busy' - if (staleReason === 'owner_busy') return 'busy' - if (error || staleReason === 'owner_unavailable') return 'unreachable' + if (error) return 'unreachable' return 'busy' } diff --git a/core/http/react-ui/src/utils/addonsApi.js b/core/http/react-ui/src/utils/addonsApi.js index ac2dd5a099..274d14d634 100644 --- a/core/http/react-ui/src/utils/addonsApi.js +++ b/core/http/react-ui/src/utils/addonsApi.js @@ -117,17 +117,12 @@ export function createAddonsApi({ fetch: fetchImpl, base = p => p } = {}) { async list(signal) { return (await api.apps.read(signal)).apps }, - // The list with the marks the server puts on it: stale is true when the - // server answers with the last list it got from the owner (the owner is - // busy or not reachable), so a page can say so and keep showing it. + // The server fails closed: when the owner does not answer it sends a + // typed 503, never an older list. A page that wants to keep showing its + // last list keeps it in its own memory. async read(signal) { const out = await call('/api/addons/apps', { signal }) - return { - apps: Array.isArray(out?.apps) ? out.apps : [], - stale: out?.stale === true, - staleReason: typeof out?.staleReason === 'string' ? out.staleReason : '', - staleSince: typeof out?.staleSince === 'string' ? out.staleSince : '', - } + return { apps: Array.isArray(out?.apps) ? out.apps : [] } }, async start(id, { operationId, generation }, signal) { const out = await call(`/api/addons/apps/${encodeURIComponent(id)}/start`, { method: 'POST', body: { operationId, generation }, signal }) diff --git a/core/http/react-ui/tests/unit/addon-session.test.mjs b/core/http/react-ui/tests/unit/addon-session.test.mjs index 44dc854b2c..983eaccd8c 100644 --- a/core/http/react-ui/tests/unit/addon-session.test.mjs +++ b/core/http/react-ui/tests/unit/addon-session.test.mjs @@ -72,7 +72,7 @@ for (const switchUser of [false, true]) test(`installed records reject late admi let auth = { user: { id: 'admin' }, isAdmin: true, sessionEpoch: 1, hasFeature: () => true } const pending = deferred() let reads = 0 - const api = { apps: { read: async () => ({ apps: [{ id: 'notes', generation: 1 }], stale: false, staleReason: '' }) }, installed: () => { reads++; return pending.promise } } + const api = { apps: { read: async () => ({ apps: [{ id: 'notes', generation: 1 }] }) }, installed: () => { reads++; return pending.promise } } const text = readFileSync(new URL('../../src/components/addons/AppsView.jsx', import.meta.url), 'utf8') // Full Apps body/effects; JSX is compiled, leaf hooks are external boundaries. const names = [...text.matchAll(/^import \{ ([^}]+) \} from/gm)].flatMap(m => m[1].split(',').map(n => n.trim())) diff --git a/core/http/react-ui/tests/unit/addonhomecommands.test.mjs b/core/http/react-ui/tests/unit/addonhomecommands.test.mjs index 57234ad86f..87ea4e1196 100644 --- a/core/http/react-ui/tests/unit/addonhomecommands.test.mjs +++ b/core/http/react-ui/tests/unit/addonhomecommands.test.mjs @@ -257,7 +257,7 @@ for (const accountSwitch of [false, true]) test(`Apps registry: late list cannot let registry = r.render() let auth = { user:{id:'alice'}, sessionEpoch:1, isAdmin:true, hasFeature:() => true } const reads = [] - const api = { apps:{ read:() => { const d = deferred(); reads.push(d); return d.promise.then(apps => ({ apps, stale:false, staleReason:'' })) } } } + const api = { apps:{ read:() => { const d = deferred(); reads.push(d); return d.promise.then(apps => ({ apps })) } } } const h = hookHarness('../../src/components/addons/AppsView.jsx', 'AppsView', { useAuth:() => auth, useContext:() => registry, RegistryContext:{}, useTranslation:() => ({t:x=>x,i18n:{}}), FEATURE_ADDONS:'addons', pollDelay:() => 1000, setTimeout:()=>1, clearTimeout:()=>{}, diff --git a/core/http/react-ui/tests/unit/addons.test.mjs b/core/http/react-ui/tests/unit/addons.test.mjs index d52e085449..68fe1828a6 100644 --- a/core/http/react-ui/tests/unit/addons.test.mjs +++ b/core/http/react-ui/tests/unit/addons.test.mjs @@ -618,16 +618,16 @@ test('addons: the details list the skills of an add-on and which replace a built assert.deepEqual(skillsOf({}), []) }) -test('addons: a busy owner is its own kind of answer, and the list read carries the stale mark', async () => { +test('addons: a busy owner is its own kind of answer, and the list read never carries a stale mark', async () => { assert.equal(classifyResponse(503, { error: { type: 'owner_busy', message: 'busy' } }).kind, 'busy') assert.equal(classifyResponse(503, { error: { type: 'owner_unavailable' } }).kind, 'unavailable') const busy = new AddonsError(classifyResponse(503, { error: { type: 'owner_busy' } })) assert.match(describeError(busy, key => key), /errors\.busy/) - const api = createAddonsApi({ fetch: async () => json(200, { apps: [{ id: 'notes' }], stale: true, staleReason: 'owner_busy', staleSince: '2026-11-04T17:18:00Z' }) }) - assert.deepEqual(await api.apps.read(), { apps: [{ id: 'notes' }], stale: true, staleReason: 'owner_busy', staleSince: '2026-11-04T17:18:00Z' }) + const api = createAddonsApi({ fetch: async () => json(200, { apps: [{ id: 'notes' }] }) }) + assert.deepEqual(await api.apps.read(), { apps: [{ id: 'notes' }] }) assert.deepEqual((await api.apps.list()).map(a => a.id), ['notes']) const plain = createAddonsApi({ fetch: async () => json(200, { apps: [] }) }) - assert.equal((await plain.apps.read()).stale, false) + assert.deepEqual(await plain.apps.read(), { apps: [] }) }) test('addons: an error is never an empty list, and the page names what the owner is upgrading', () => { @@ -644,9 +644,8 @@ test('addons: an error is never an empty list, and the page names what the owner assert.deepEqual(operationOf({ operation: { kind: 'defrag', phase: 'later' } }), { kind: 'working', phase: 'working', since: '' }) assert.equal(staleKind({ stale: false, error: new Error('x') }), '') assert.equal(staleKind({ stale: true, error: { kind: 'busy' } }), 'busy') - assert.equal(staleKind({ stale: true, staleReason: 'owner_busy' }), 'busy') + assert.equal(staleKind({ stale: true }), 'busy') assert.equal(staleKind({ stale: true, error: { kind: 'network' } }), 'unreachable') - assert.equal(staleKind({ stale: true, staleReason: 'owner_unavailable' }), 'unreachable') }) test('addons: the last list of a tab stands in for a page loaded while the owner is busy', () => { diff --git a/core/services/addons/apps/apps.go b/core/services/addons/apps/apps.go index 4dde6450ba..bb8c650914 100644 --- a/core/services/addons/apps/apps.go +++ b/core/services/addons/apps/apps.go @@ -61,16 +61,6 @@ type Operation struct { // List is the response of the apps list. type List struct { Apps []App `json:"apps"` - // Stale is true when the receiving frontend could not reach the owner in - // time and answers with the last list it received from it. The list may be - // out of date; Apps still carries the operations the owner reported then. - Stale bool `json:"stale,omitempty"` - // StaleReason is why the list is stale: owner_busy when the owner did not - // answer within the read deadline, owner_unavailable when it could not be - // reached at all. - StaleReason string `json:"staleReason,omitempty" enums:"owner_busy,owner_unavailable"` - // StaleSince is when the owner last answered, RFC 3339 in UTC. - StaleSince string `json:"staleSince,omitempty"` } // StartRequest is the body of the start and stop routes. The generation is diff --git a/core/services/addons/gateway/apps.go b/core/services/addons/gateway/apps.go index 6da1b94be8..6df6350e59 100644 --- a/core/services/addons/gateway/apps.go +++ b/core/services/addons/gateway/apps.go @@ -6,15 +6,11 @@ import ( "crypto/rand" "crypto/sha256" "encoding/hex" - "errors" "fmt" "io" "net/http" "os" - "slices" "strconv" - "sync" - "time" "github.com/mudler/LocalAI/core/services/addons/apps" "github.com/mudler/LocalAI/pkg/addons/contract" @@ -34,92 +30,17 @@ func (g *Gateway) Apps(ctx context.Context, a contract.Actor) (apps.List, error) return apps.List{}, e } if g.Forward != nil { - list, e := g.Forward.apps(ctx, a) - if e == nil { - g.appsCache.put(a, list) - return list, nil - } - // The owner is busy or unreachable: a page is better served with the - // last list this frontend got for the same person, marked stale, than - // with an error it could mistake for "nothing is installed". A denial - // is never covered by the cache. - if StatusFor(e) == http.StatusServiceUnavailable && ctx.Err() == nil { - if old, ok := g.appsCache.get(a); ok { - var timeout *OwnerTimeout - old.list.Stale, old.list.StaleReason = true, "owner_unavailable" - if errors.As(e, &timeout) { - old.list.StaleReason = "owner_busy" - } - old.list.StaleSince = old.at.UTC().Format(time.RFC3339) - return old.list, nil - } - } - return apps.List{}, e + // The owner's answer is the answer. When the owner cannot answer in + // time, every frontend gives the same typed error (OwnerTimeout is + // owner_busy, anything else owner_unavailable) and never an older list + // of its own: a per-frontend copy would make two frontends disagree + // about whether the owner answered. A page keeps the last list in the + // browser. + return g.Forward.apps(ctx, a) } return g.appsLocal(ctx, a) } -// AppsStaleTTL is how long a frontend keeps the last apps list of a person to -// answer with while the owner does not. -const AppsStaleTTL = 10 * time.Minute - -const appsCacheMax = 256 - -type cachedApps struct { - list apps.List - at time.Time -} - -// appsCache holds the last good list per person. It is a per-process -// convenience for the busy owner, never a source of truth: it is only read -// when the owner cannot answer, and it expires. -type appsCache struct { - mu sync.Mutex - entries map[string]cachedApps - now func() time.Time -} - -func cacheKey(a contract.Actor) string { return a.Mode + "\x00" + a.UserID } - -func (c *appsCache) clock() time.Time { - if c.now != nil { - return c.now() - } - return time.Now() -} - -func (c *appsCache) put(a contract.Actor, list apps.List) { - c.mu.Lock() - defer c.mu.Unlock() - if c.entries == nil { - c.entries = map[string]cachedApps{} - } - if _, ok := c.entries[cacheKey(a)]; !ok && len(c.entries) >= appsCacheMax { - oldest, when := "", time.Time{} - for k, v := range c.entries { - if oldest == "" || v.at.Before(when) { - oldest, when = k, v.at - } - } - delete(c.entries, oldest) - } - list.Stale, list.StaleReason, list.StaleSince = false, "", "" - c.entries[cacheKey(a)] = cachedApps{list: list, at: c.clock()} -} - -func (c *appsCache) get(a contract.Actor) (cachedApps, bool) { - c.mu.Lock() - defer c.mu.Unlock() - e, ok := c.entries[cacheKey(a)] - if !ok || c.clock().Sub(e.at) > AppsStaleTTL { - delete(c.entries, cacheKey(a)) - return cachedApps{}, false - } - // The apps slice is shared with later readers: hand out a copy of it. - e.list.Apps = slices.Clone(e.list.Apps) - return e, true -} - func (g *Gateway) appsLocal(ctx context.Context, a contract.Actor) (apps.List, error) { if g.Manager == nil { return apps.List{}, ErrUnavailable diff --git a/core/services/addons/gateway/busyowner_test.go b/core/services/addons/gateway/busyowner_test.go index a69e6e7e6c..150dfc9928 100644 --- a/core/services/addons/gateway/busyowner_test.go +++ b/core/services/addons/gateway/busyowner_test.go @@ -14,7 +14,6 @@ import ( var _ = Describe("apps list while the owner is busy", func() { alice := contract.Actor{UserID: "alice", CredentialID: "key:alice", Mode: "named-user"} - bob := contract.Actor{UserID: "bob", CredentialID: "key:bob", Mode: "named-user"} allow := func(context.Context, contract.Actor) error { return nil } // busyOwner serves the apps list until stall is set, then holds every @@ -53,43 +52,28 @@ var _ = Describe("apps list while the owner is busy", func() { Expect(e).To(MatchError(ErrUnavailable)) }) - It("answers with the last good list marked stale, and only for the same person", func() { + It("never answers with an older list of its own: every frontend gives the same typed error", func() { front, _, stall := busyOwner() fresh, e := front.Apps(context.Background(), alice) Expect(e).NotTo(HaveOccurred()) - Expect(fresh.Stale).To(BeFalse()) Expect(fresh.Apps).NotTo(BeEmpty()) + // This frontend has a good list of alice. A frontend that never read + // one must not differ from it, so the first one gives no list either. stall.Store(true) list, e := front.Apps(context.Background(), alice) - Expect(e).NotTo(HaveOccurred()) - Expect(list.Stale).To(BeTrue()) - Expect(list.StaleReason).To(Equal("owner_busy")) - Expect(list.StaleSince).NotTo(BeEmpty()) - Expect(list.Apps).To(HaveLen(len(fresh.Apps))) - - _, e = front.Apps(context.Background(), bob) - Expect(e).To(HaveOccurred(), "another person never gets alice's list") + Expect(e).To(HaveOccurred()) + Expect(list.Apps).To(BeEmpty()) + var timeout *OwnerTimeout + Expect(e).To(BeAssignableToTypeOf(timeout)) stall.Store(false) again, e := front.Apps(context.Background(), alice) Expect(e).NotTo(HaveOccurred()) - Expect(again.Stale).To(BeFalse()) + Expect(again.Apps).To(HaveLen(len(fresh.Apps))) }) - It("lets a cached list expire", func() { - front, _, stall := busyOwner() - now := time.Now() - front.appsCache.now = func() time.Time { return now } - _, e := front.Apps(context.Background(), alice) - Expect(e).NotTo(HaveOccurred()) - stall.Store(true) - now = now.Add(AppsStaleTTL + time.Second) - _, e = front.Apps(context.Background(), alice) - Expect(e).To(HaveOccurred()) - }) - - It("does not hide a denial behind the cache", func() { + It("does not hide a denial", func() { front, _, _ := busyOwner() _, e := front.Apps(context.Background(), alice) Expect(e).NotTo(HaveOccurred()) diff --git a/core/services/addons/gateway/gateway.go b/core/services/addons/gateway/gateway.go index a2b1570ddc..73bb2e9951 100644 --- a/core/services/addons/gateway/gateway.go +++ b/core/services/addons/gateway/gateway.go @@ -26,8 +26,6 @@ var ErrNotSent = fmt.Errorf("the add-on is not running now and nothing was sent: // Gateway deliberately admits lifecycle commands only until resource admission // and proposal confirmation are installed. Installation admin is not data access. type Gateway struct { - // appsCache backs the stale answer of the apps list; see Apps. - appsCache appsCache // Transfers holds the handles of connector file transfers on the process // that runs the add-ons. Without it a call that carries a handle is // unavailable. diff --git a/docs/content/features/addons.md b/docs/content/features/addons.md index 7f5d1373bd..0638ef0230 100644 --- a/docs/content/features/addons.md +++ b/docs/content/features/addons.md @@ -950,21 +950,24 @@ the apps list: still those of the running add-on until the new generation is published. A frontend that forwards a read to the owner waits at most eight seconds. When -the owner does not answer in that time: +the owner does not answer in that time, the route fails closed with the same +typed `503` from every frontend, and never with an older list of its own: -- The apps list answers `200` with `"stale": true`, `"staleReason"` (`owner_busy` - or `owner_unavailable`) and `"staleSince"`, and carries the last list this - frontend received for the same person. The copy lives for ten minutes and is - never used for a refused request. -- Without such a copy the route answers `503` with the error type `owner_busy` - (the owner did not answer in time) or `owner_unavailable`. `GET /api/addons/pins` - does the same instead of returning pins without the add-ons it could not check. +- `owner_busy`: the owner is up but did not answer in time. +- `owner_unavailable`: the owner could not be reached at all. -The Add-ons page keeps showing the last list, shows a notice such as "The -add-on server is busy (upgrading Sandbox). Showing the last known state.", marks -each changing add-on as upgrading with its phase, and asks again with a growing -delay. It only says that no add-ons are installed when the owner answered with -an empty list. +A frontend keeps no copy of the apps list, because two frontends with different +copies would give different answers for the same request. `GET /api/addons/pins` +answers the same way instead of returning pins without the add-ons it could not +check. When the owner is reachable and busy, it still answers from its snapshot, +so the apps list is a normal `200` with the `operation` fields above. + +The Add-ons page keeps the last list it received in the browser (in memory and, +for a page loaded during an upgrade, in the session storage of the tab). On a +`503` it keeps showing that list, shows a notice such as "The add-on server is +busy. Showing the last known state.", and asks again with a growing delay. It +only says that no add-ons are installed when the owner answered with an empty +list. ## The official library diff --git a/swagger/docs.go b/swagger/docs.go index b540e05bc2..7633b2006f 100644 --- a/swagger/docs.go +++ b/swagger/docs.go @@ -164,7 +164,7 @@ const docTemplate = `{ } }, "503": { - "description": "Addons or the owner are unavailable, or the owner did not answer in time and this frontend has no earlier list to show (type owner_busy or owner_unavailable)", + "description": "Addons or the owner are unavailable, or the owner did not answer in time (type owner_busy or owner_unavailable)", "schema": { "$ref": "#/definitions/addons.AddonErrorResponse" } @@ -16398,22 +16398,6 @@ const docTemplate = `{ "items": { "$ref": "#/definitions/apps.App" } - }, - "stale": { - "description": "Stale is true when the receiving frontend could not reach the owner in\ntime and answers with the last list it received from it. The list may be\nout of date; Apps still carries the operations the owner reported then.", - "type": "boolean" - }, - "staleReason": { - "description": "StaleReason is why the list is stale: owner_busy when the owner did not\nanswer within the read deadline, owner_unavailable when it could not be\nreached at all.", - "type": "string", - "enum": [ - "owner_busy", - "owner_unavailable" - ] - }, - "staleSince": { - "description": "StaleSince is when the owner last answered, RFC 3339 in UTC.", - "type": "string" } } }, diff --git a/swagger/swagger.json b/swagger/swagger.json index d2829fd4fc..eb86cb5073 100644 --- a/swagger/swagger.json +++ b/swagger/swagger.json @@ -161,7 +161,7 @@ } }, "503": { - "description": "Addons or the owner are unavailable, or the owner did not answer in time and this frontend has no earlier list to show (type owner_busy or owner_unavailable)", + "description": "Addons or the owner are unavailable, or the owner did not answer in time (type owner_busy or owner_unavailable)", "schema": { "$ref": "#/definitions/addons.AddonErrorResponse" } @@ -16395,22 +16395,6 @@ "items": { "$ref": "#/definitions/apps.App" } - }, - "stale": { - "description": "Stale is true when the receiving frontend could not reach the owner in\ntime and answers with the last list it received from it. The list may be\nout of date; Apps still carries the operations the owner reported then.", - "type": "boolean" - }, - "staleReason": { - "description": "StaleReason is why the list is stale: owner_busy when the owner did not\nanswer within the read deadline, owner_unavailable when it could not be\nreached at all.", - "type": "string", - "enum": [ - "owner_busy", - "owner_unavailable" - ] - }, - "staleSince": { - "description": "StaleSince is when the owner last answered, RFC 3339 in UTC.", - "type": "string" } } }, diff --git a/swagger/swagger.yaml b/swagger/swagger.yaml index 3876f0b95b..8455a0907b 100644 --- a/swagger/swagger.yaml +++ b/swagger/swagger.yaml @@ -536,24 +536,6 @@ definitions: items: $ref: '#/definitions/apps.App' type: array - stale: - description: |- - Stale is true when the receiving frontend could not reach the owner in - time and answers with the last list it received from it. The list may be - out of date; Apps still carries the operations the owner reported then. - type: boolean - staleReason: - description: |- - StaleReason is why the list is stale: owner_busy when the owner did not - answer within the read deadline, owner_unavailable when it could not be - reached at all. - enum: - - owner_busy - - owner_unavailable - type: string - staleSince: - description: StaleSince is when the owner last answered, RFC 3339 in UTC. - type: string type: object apps.NetworkView: properties: @@ -12091,8 +12073,7 @@ paths: $ref: '#/definitions/addons.AddonErrorResponse' "503": description: Addons or the owner are unavailable, or the owner did not answer - in time and this frontend has no earlier list to show (type owner_busy - or owner_unavailable) + in time (type owner_busy or owner_unavailable) schema: $ref: '#/definitions/addons.AddonErrorResponse' summary: List the addon apps the caller may use