mirror of
https://github.com/mudler/LocalAI.git
synced 2026-11-06 04:23:15 -05:00
fix(addons): fail closed on the apps list when the owner does not answer
A frontend kept the last apps list per person and answered 200 with a stale mark when the owner timed out. Two frontends then disagreed: one with a cached list answered 200, the other a typed 503. The acceptance contract is that every frontend gives the same typed answer when the owner cannot answer, so the server cache is gone. The route now answers owner_busy (timeout) or owner_unavailable with a 503 from every frontend. An owner that is up and busy still answers from its snapshot, with the operation of each add-on in the list. The Add-ons page keeps the last list in the browser and shows it with the busy notice, so it never goes empty. The specs, the docs and the swagger spec follow the new contract. Assisted-by: Claude:claude-sonnet-5-5
This commit is contained in:
17 files changed
+75
-235
No files matched your search
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
|
||||
@@ -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),
|
||||
|
||||
+3
-5
@@ -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'
|
||||
}
|
||||
|
||||
|
||||
+4
-9
@@ -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 })
|
||||
|
||||
@@ -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()))
|
||||
|
||||
@@ -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:()=>{},
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
+1
-17
@@ -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"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
+1
-17
@@ -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"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
+1
-20
@@ -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
|
||||
|
||||
Reference in new issue
Block a user