Stop shipping a placeholder that looks like a value, and say what is served
CI / test (push) Successful in 1m23s
CI / test (push) Successful in 1m23s
A user copied the curl from the profile page and sent it as it stood. Every other field in that snippet is filled in for them — the base URL, the key they had just been issued — so "model": "MODEL" reads as a value rather than a blank, and they got: no endpoint serves "MODEL" which is true and useless. It reads the same whether the name was a typo, a placeholder left in from the documentation, or a real outage, and it never says what would have worked. The message now lists what is being served, capped at ten with a count for the rest, and says plainly when nothing is. That list is what /v1/models already returns to the same caller, so it discloses nothing new. The profile page's example now names a model that is actually running, real ones before simulated — the point of an example someone can run is that it runs, and sending a first request to the sim engine would answer in Greek letters. The landing and claim pages keep a placeholder, because they are served without a key and deliberately say nothing about what is here; theirs is <model-name>, which cannot be mistaken for a value, with a line saying where to get the real one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
6f3bde099e
commit
082d52bc97
8 files changed
+170
-6
No files matched your search
@@ -5,6 +5,7 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"slices"
|
||||
@@ -107,6 +108,9 @@ func (r *Router) Handler() http.Handler {
|
||||
// authenticated by a session cookie, not by an API key, which is why
|
||||
// they are exempt below.
|
||||
pages := web.NewAccountPages(r.accounts, r.store, r.store, version.Version, r.log)
|
||||
// So the example on the profile page names a model that is actually
|
||||
// running, rather than a placeholder that gets sent as-is.
|
||||
pages.ServedModels(r.exampleModels)
|
||||
pages.Register(mux)
|
||||
r.registerChat(mux, pages)
|
||||
|
||||
@@ -460,6 +464,51 @@ func injectIncludeUsage(body []byte) ([]byte, bool) {
|
||||
return out, true
|
||||
}
|
||||
|
||||
// noEndpointFor explains that nothing serves a model, and says what does.
|
||||
//
|
||||
// "no endpoint serves \"MODEL\"" was all a client got, which is true and
|
||||
// useless: it reads the same whether the name was a typo, a placeholder left
|
||||
// in from the documentation, or a real outage. Someone copying the curl from
|
||||
// the account page sent the literal word MODEL and had nothing to go on.
|
||||
//
|
||||
// The list is what /v1/models already returns to the same caller, so naming it
|
||||
// here discloses nothing new. It is capped because a large catalog would turn
|
||||
// an error message into a page.
|
||||
func noEndpointFor(model string, served []string) string {
|
||||
if len(served) == 0 {
|
||||
return fmt.Sprintf("no endpoint serves %q, and no model is currently being served", model)
|
||||
}
|
||||
const showAt = 10
|
||||
shown := served
|
||||
suffix := ""
|
||||
if len(shown) > showAt {
|
||||
shown, suffix = shown[:showAt], fmt.Sprintf(" and %d more", len(served)-showAt)
|
||||
}
|
||||
return fmt.Sprintf("no endpoint serves %q; available models are %s%s",
|
||||
model, strings.Join(shown, ", "), suffix)
|
||||
}
|
||||
|
||||
// exampleModels is servedModels with the ones that answer for real first.
|
||||
//
|
||||
// The profile page takes the first name for its copy-pasteable example, and
|
||||
// handing it a simulated model would send someone's first request to an engine
|
||||
// that replies in Greek letters.
|
||||
func (r *Router) exampleModels() []string {
|
||||
names := r.servedModels()
|
||||
slices.SortStableFunc(names, func(a, b string) int {
|
||||
as, bs := r.models[a].Simulated(), r.models[b].Simulated()
|
||||
switch {
|
||||
case as == bs:
|
||||
return 0
|
||||
case bs:
|
||||
return -1
|
||||
default:
|
||||
return 1
|
||||
}
|
||||
})
|
||||
return names
|
||||
}
|
||||
|
||||
// servedModels lists the canonical models at least one endpoint serves.
|
||||
func (r *Router) servedModels() []string {
|
||||
seen := map[string]struct{}{}
|
||||
|
||||
@@ -419,7 +419,7 @@ var errNoCapacity = errors.New("no endpoint available")
|
||||
func (r *Router) dispatchOne(ctx context.Context, w http.ResponseWriter, req *parsedRequest, model string, entry catalog.Model) dispatchResult {
|
||||
candidates := r.reg.Snapshots(model)
|
||||
if len(candidates) == 0 {
|
||||
return dispatchResult{err: fmt.Errorf("%w: no endpoint serves %q", errNoCapacity, model)}
|
||||
return dispatchResult{err: fmt.Errorf("%w: %s", errNoCapacity, noEndpointFor(model, r.servedModels()))}
|
||||
}
|
||||
|
||||
profile := r.profileFor(entry, req.ProfileOverride)
|
||||
|
||||
@@ -363,3 +363,55 @@ func TestSessionHeaderFallsBackToUser(t *testing.T) {
|
||||
t.Errorf("the header should win, got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUnknownModelErrorNamesWhatIsAvailable(t *testing.T) {
|
||||
// Reported by a user who copied the curl from the account page and sent
|
||||
// its placeholder unchanged. "no endpoint serves \"MODEL\"" is true and
|
||||
// useless: it reads the same for a typo, a placeholder and a real outage.
|
||||
e := newFakeEngine(t, "m")
|
||||
rt := newTestRouter(t, baseConfig(config.Endpoint{ID: "a", URL: e.srv.URL, Models: []string{"m"}}))
|
||||
|
||||
rec := post(t, rt.Handler(), map[string]any{"model": "MODEL", "messages": []any{}}, nil)
|
||||
if rec.Code != http.StatusServiceUnavailable {
|
||||
t.Fatalf("status %d, expected 503", rec.Code)
|
||||
}
|
||||
var body struct {
|
||||
Error struct {
|
||||
Message string `json:"message"`
|
||||
} `json:"error"`
|
||||
}
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
|
||||
t.Fatalf("error body is not JSON: %q", rec.Body.String())
|
||||
}
|
||||
if !strings.Contains(body.Error.Message, `"MODEL"`) {
|
||||
t.Errorf("the message does not repeat what was asked for: %q", body.Error.Message)
|
||||
}
|
||||
if !strings.Contains(body.Error.Message, "m") ||
|
||||
!strings.Contains(body.Error.Message, "available") {
|
||||
t.Errorf("the message does not say what is available: %q", body.Error.Message)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTheErrorDoesNotBecomeAPage(t *testing.T) {
|
||||
// A large catalog must not turn one error into a wall of names.
|
||||
var many []string
|
||||
for i := range 40 {
|
||||
many = append(many, fmt.Sprintf("model-%02d", i))
|
||||
}
|
||||
msg := noEndpointFor("nope", many)
|
||||
if !strings.Contains(msg, "and 30 more") {
|
||||
t.Errorf("the list was not capped: %q", msg)
|
||||
}
|
||||
if strings.Contains(msg, "model-11") {
|
||||
t.Errorf("more than the cap was listed: %q", msg)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTheErrorIsHonestWhenNothingIsServed(t *testing.T) {
|
||||
// Listing nothing as "available models are " would read as a truncated
|
||||
// message rather than an empty cluster.
|
||||
msg := noEndpointFor("m", nil)
|
||||
if !strings.Contains(msg, "no model is currently being served") {
|
||||
t.Errorf("an empty fleet is not explained: %q", msg)
|
||||
}
|
||||
}
|
||||
+34
-2
@@ -47,6 +47,33 @@ type AccountPages struct {
|
||||
passwords account.PasswordChanger
|
||||
version string
|
||||
log *slog.Logger
|
||||
// served, when set, names the models an endpoint is currently serving. The
|
||||
// curl example on this page fills its model field from it: every other
|
||||
// field in that snippet is real, so the one placeholder left in it was
|
||||
// copied verbatim and sent as the literal string MODEL.
|
||||
served func() []string
|
||||
}
|
||||
|
||||
// ServedModels supplies the names the profile page's example should use.
|
||||
//
|
||||
// A setter rather than a constructor argument because only the router knows
|
||||
// what is being served, and the page is useful without it — with no source the
|
||||
// example falls back to a placeholder, which is what it always was.
|
||||
func (p *AccountPages) ServedModels(f func() []string) { p.served = f }
|
||||
|
||||
// exampleModel picks the model the copy-pasteable example should name.
|
||||
//
|
||||
// A real model over a simulated one is not expressible here — this package does
|
||||
// not know which is which — so it takes the first served name and relies on the
|
||||
// router listing real models first.
|
||||
func (p *AccountPages) exampleModel() string {
|
||||
if p.served == nil {
|
||||
return "<model-name>"
|
||||
}
|
||||
if names := p.served(); len(names) > 0 {
|
||||
return names[0]
|
||||
}
|
||||
return "<model-name>"
|
||||
}
|
||||
|
||||
// NewAccountPages builds the handlers.
|
||||
@@ -189,8 +216,11 @@ type profileData struct {
|
||||
|
||||
CSRF string
|
||||
BaseURL string
|
||||
Version string
|
||||
Now string
|
||||
// ExampleModel is a model that is actually being served, so the example on
|
||||
// the page can be copied and run as it stands.
|
||||
ExampleModel string
|
||||
Version string
|
||||
Now string
|
||||
}
|
||||
|
||||
func (p *AccountPages) profile(w http.ResponseWriter, r *http.Request) {
|
||||
@@ -272,6 +302,7 @@ func (p *AccountPages) render(w http.ResponseWriter, r *http.Request,
|
||||
d.User = user
|
||||
d.MinPassword = account.MinPasswordLen
|
||||
d.BaseURL = baseURL(r)
|
||||
d.ExampleModel = p.exampleModel()
|
||||
d.Version = p.version
|
||||
d.Now = time.Now().UTC().Format(time.RFC3339)
|
||||
// The form token comes from the cookie, never from whatever the request
|
||||
@@ -474,6 +505,7 @@ func (p *AccountPages) renderWithCSRF(w http.ResponseWriter, r *http.Request,
|
||||
d.User = user
|
||||
d.MinPassword = account.MinPasswordLen
|
||||
d.BaseURL = baseURL(r)
|
||||
d.ExampleModel = p.exampleModel()
|
||||
d.Version = p.version
|
||||
d.Now = time.Now().UTC().Format(time.RFC3339)
|
||||
d.CSRF = csrf
|
||||
|
||||
@@ -0,0 +1,21 @@
|
||||
package web
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestTheExampleNamesARealModel(t *testing.T) {
|
||||
// Every other field in that curl is filled in — the base URL, the key —
|
||||
// so the one placeholder in it reads like a value. It was copied verbatim
|
||||
// and sent as the literal string MODEL.
|
||||
p := &AccountPages{}
|
||||
if got := p.exampleModel(); got != "<model-name>" {
|
||||
t.Errorf("with no source the example should be an obvious placeholder, got %q", got)
|
||||
}
|
||||
p.ServedModels(func() []string { return nil })
|
||||
if got := p.exampleModel(); got != "<model-name>" {
|
||||
t.Errorf("with nothing served the example should be a placeholder, got %q", got)
|
||||
}
|
||||
p.ServedModels(func() []string { return []string{"qwen", "big-model"} })
|
||||
if got := p.exampleModel(); got != "qwen" {
|
||||
t.Errorf("example model = %q, want the first served name", got)
|
||||
}
|
||||
}
|
||||
@@ -73,7 +73,7 @@ Tell whoever you asked for access that you have signed up.</p>
|
||||
<pre>curl {{.BaseURL}}/v1/chat/completions \
|
||||
-H "Authorization: Bearer {{.NewKey}}" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d '{"model": "MODEL", "messages": [{"role":"user","content":"hello"}]}'</pre>
|
||||
-d '{"model": "{{.ExampleModel}}", "messages": [{"role":"user","content":"hello"}]}'</pre>
|
||||
{{end}}
|
||||
|
||||
<h2>Create a key</h2>
|
||||
|
||||
@@ -20,7 +20,12 @@
|
||||
curl {{.BaseURL}}/v1/chat/completions \
|
||||
-H "Authorization: Bearer $LLMGW_API_KEY" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d '{"model": "MODEL", "messages": [{"role":"user","content":"hello"}]}'</pre>
|
||||
-d '{"model": "<model-name>", "messages": [{"role":"user","content":"hello"}]}'</pre>
|
||||
<p class="note">
|
||||
Replace <model-name> with a name from
|
||||
<code>GET {{.BaseURL}}/v1/models</code>, which needs your key. Sending the
|
||||
placeholder unchanged is answered with the list of models that are available.
|
||||
</p>
|
||||
<pre>from openai import OpenAI
|
||||
client = OpenAI(base_url="{{.BaseURL}}/v1", api_key=os.environ["LLMGW_API_KEY"])</pre>
|
||||
<p class="note"><code>GET {{.BaseURL}}/v1/models</code> lists what this key may use.
|
||||
|
||||
@@ -18,7 +18,12 @@ client for; only the base URL changes.</p>
|
||||
<pre>curl {{.BaseURL}}/v1/chat/completions \
|
||||
-H "Authorization: Bearer $LLMGW_API_KEY" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d '{"model": "MODEL", "messages": [{"role":"user","content":"hello"}]}'</pre>
|
||||
-d '{"model": "<model-name>", "messages": [{"role":"user","content":"hello"}]}'</pre>
|
||||
<p class="note">
|
||||
Replace <model-name> with a name from
|
||||
<code>GET {{.BaseURL}}/v1/models</code>, which needs your key. Sending the
|
||||
placeholder unchanged is answered with the list of models that are available.
|
||||
</p>
|
||||
|
||||
<pre>from openai import OpenAI
|
||||
client = OpenAI(base_url="{{.BaseURL}}/v1", api_key=os.environ["LLMGW_API_KEY"])</pre>
|
||||
|
||||
Reference in new issue
Block a user