diff --git a/internal/httpapi/auth_handlers.go b/internal/httpapi/auth_handlers.go index 57d6508..7ab3ab0 100644 --- a/internal/httpapi/auth_handlers.go +++ b/internal/httpapi/auth_handlers.go @@ -573,6 +573,34 @@ func inboxName(locale string) string { // 11:49" long after the line explaining it has gone, which tells an operator that // something broke and gives them no way to find out what. The row outlives the // container; Indstillinger → Fejl is where it is read. +// upstream reports a failure that came from somebody else's service. +// +// Separate from `internal` because the cause is different and so is the fix: a 500 +// is this server broken, and this is Google — or whoever — saying no. The status +// says so (502), and the message is passed through rather than replaced, because +// "invalid_grant" or "insufficient authentication scopes" is the whole diagnosis +// and a generic sentence in its place throws it away. +// +// It is recorded like a 500 for exactly the reason the error log exists: a Gmail +// sync that has been failing for a week is invisible otherwise. The background job +// writes a log line the next restart erases, and nobody is watching the one screen +// that would have said so. +func (s *Server) upstream(w http.ResponseWriter, r *http.Request, code, what string, err error) { + s.log.Warn(what, "err", err) + + userID := "" + if u := userFrom(r.Context()); u != nil { + userID = u.ID + } + s.db.RecordError(r.Context(), store.ServerError{ + Method: r.Method, Path: r.URL.Path, Status: http.StatusBadGateway, + What: what, Message: err.Error(), UserID: userID, + RequestID: middleware.GetReqID(r.Context()), + }) + + writeError(w, http.StatusBadGateway, code, err.Error()) +} + func (s *Server) internal(w http.ResponseWriter, r *http.Request, what string, err error) { s.log.Error(what, "err", err) diff --git a/internal/httpapi/errors.go b/internal/httpapi/errors.go index 7295314..1be61b5 100644 --- a/internal/httpapi/errors.go +++ b/internal/httpapi/errors.go @@ -51,6 +51,13 @@ const ( // is not "that already exists" either, and it is the one refusal on the user // page that a person needs explained rather than merely reported. CodeLastAdmin = "last_admin" + + // Gmail said no. Its own words are the diagnosis — "invalid_grant", + // "insufficient authentication scopes" — and they are useless behind a generic + // sentence, which is what `internal_error` gave: a person clicked Fetch now, + // read "something went wrong", and had nowhere to go. The interface wraps the + // message in one that says where it came from. + CodeGmailFailed = "gmail_failed" ) // errLastAdmin is returned to the caller that already wrote the response, so it diff --git a/internal/httpapi/gmail_handlers.go b/internal/httpapi/gmail_handlers.go index 4979029..c1df767 100644 --- a/internal/httpapi/gmail_handlers.go +++ b/internal/httpapi/gmail_handlers.go @@ -150,6 +150,9 @@ func (s *Server) handleGmailCallback(w http.ResponseWriter, r *http.Request) { return } + // Best effort: a mailbox that connects but cannot name itself is still + // connected, and the status endpoint asks again when it finds no address — + // so this failing is a delay rather than a dead end. email, err := gmail.NewClient(token.AccessToken).Profile(r.Context()) if err != nil { s.log.Warn("gmail profile", "err", err) @@ -184,7 +187,7 @@ func (s *Server) handleGmailSyncNow(w http.ResponseWriter, r *http.Request) { created, err := s.SyncGmail(r.Context(), user) if err != nil { - writeError(w, http.StatusBadGateway, CodeInternal, err.Error()) + s.upstream(w, r, CodeGmailFailed, "sync gmail", err) return } writeJSON(w, http.StatusOK, map[string]int{"created": created}) @@ -425,3 +428,39 @@ func (s *Server) handleSetGmailClient(w http.ResponseWriter, r *http.Request) { } w.WriteHeader(http.StatusNoContent) } + +// gmailProfile fetches the mailbox address and stores it, refreshing the access +// token first if it has expired. +// +// Shared by the connect callback and the status endpoint. The callback is the +// natural place to learn the address, and it is also the one moment that cannot be +// retried — so the status endpoint asks again when it finds nothing, and the answer +// corrects itself on the next look. +func (s *Server) gmailProfile(ctx context.Context, user *store.User, settings map[string]any) (string, error) { + str := func(key string) string { + v, _ := settings[key].(string) + return v + } + + access := str("access_token") + expires, _ := settings["expires_at"].(float64) + if access == "" || int64(expires) <= time.Now().Unix() { + token, err := s.gmailConfig(ctx).Refresh(ctx, str("refresh_token")) + if err != nil { + return "", err + } + access = token.AccessToken + settings["access_token"] = access + settings["expires_at"] = token.ExpiresAt.Unix() + } + + email, err := gmail.NewClient(access).Profile(ctx) + if err != nil { + return "", err + } + settings["email"] = email + if err := s.db.SetUserSettings(ctx, user.ID, "gmail", settings); err != nil { + return "", err + } + return email, nil +} diff --git a/internal/httpapi/gmail_test.go b/internal/httpapi/gmail_test.go new file mode 100644 index 0000000..74ed08d --- /dev/null +++ b/internal/httpapi/gmail_test.go @@ -0,0 +1,70 @@ +package httpapi + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/kristianwind/verdande/internal/config" +) + +// A Gmail failure has to survive being reported. +// +// It used to be hidden twice over. The handler answered 502 with the code +// `internal_error`, which the interface maps to "something went wrong" — so +// Google's own words, the only thing that says what to do, were replaced by a +// sentence that says nothing. And `writeError` records nothing, so the error log +// was empty: a person clicked Fetch now, read a generic failure, and there was +// nowhere to look it up afterwards. +func TestAGmailFailureSaysWhatGmailSaidAndIsRecorded(t *testing.T) { + // A Gmail that refuses, the way an expired grant does. + google := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{"error":"invalid_grant"}`, http.StatusBadRequest) + })) + defer google.Close() + + ts := newTestServerWith(t, func(cfg *config.Config) { + cfg.GmailClientID = "test-client" + cfg.GmailClientSecret = "test-secret" + }) + ts.bootstrap(t) + + user, err := ts.db.UserByEmail(t.Context(), "kristian@example.dk") + if err != nil { + t.Fatal(err) + } + // Connected, with a refresh token that will be refused. + if err := ts.db.SetUserSettings(t.Context(), user.ID, "gmail", map[string]any{ + "refresh_token": "spent", "trigger": "starred", + }); err != nil { + t.Fatal(err) + } + + resp, body := ts.do(t, "POST", "/api/v1/gmail/sync", nil) + if resp.StatusCode != http.StatusBadGateway { + t.Fatalf("sync: status %d, want 502 — this is Gmail saying no, not this server breaking", + resp.StatusCode) + } + if body["code"] != "gmail_failed" { + t.Errorf("code = %v, want gmail_failed — `internal_error` is mapped to a generic "+ + "sentence, which throws the diagnosis away", body["code"]) + } + if msg, _ := body["error"].(string); msg == "" { + t.Error("the message is empty; Gmail's own words are the whole diagnosis") + } + + // And it is in the error log, which is the point of having one: a sync that has + // been failing for a week is invisible otherwise. + _, logged := ts.do(t, "GET", "/api/v1/errors", nil) + rows, _ := logged["errors"].([]any) + if len(rows) == 0 { + t.Fatal("nothing was recorded; the failure is as invisible as it was before") + } + row := rows[0].(map[string]any) + if row["what"] != "sync gmail" { + t.Errorf("what = %v, want \"sync gmail\"", row["what"]) + } + if row["status"] != float64(http.StatusBadGateway) { + t.Errorf("status = %v, want 502", row["status"]) + } +} diff --git a/internal/httpapi/integrations_handlers.go b/internal/httpapi/integrations_handlers.go index 02e5a5f..008407d 100644 --- a/internal/httpapi/integrations_handlers.go +++ b/internal/httpapi/integrations_handlers.go @@ -379,9 +379,27 @@ func (s *Server) handleGetGmail(w http.ResponseWriter, r *http.Request) { v, _ := values[key].(string) return v } + // The mailbox address, fetched again if it is missing. + // + // It is stored at connect time, and the call that fetches it can fail — a + // slow token, a scope Google had not applied yet. That failure used to be a + // log line and an empty string, so the page said "an unknown account" and went + // on saying it forever: the only way back was to disconnect and reconnect, + // which nobody would guess. Asking again here costs one request on a page + // nobody opens often, and it means the wrong answer lasts until the next look + // rather than until somebody reconnects. + email := str("email") + if email == "" && str("refresh_token") != "" { + if fetched, err := s.gmailProfile(r.Context(), userFrom(r.Context()), values); err == nil { + email = fetched + } else { + s.log.Warn("gmail profile", "err", err) + } + } + writeJSON(w, http.StatusOK, gmailSettings{ Connected: str("refresh_token") != "", - Email: str("email"), + Email: email, Trigger: str("trigger"), Label: str("label"), }) diff --git a/internal/i18n/untranslated_test.go b/internal/i18n/untranslated_test.go index c68d2a4..6622c26 100644 --- a/internal/i18n/untranslated_test.go +++ b/internal/i18n/untranslated_test.go @@ -20,6 +20,56 @@ import ( // away whatever letters it happens to use. A word list rather than a language // detector: this repository ships one static binary, and a list of thirty common // words catches the sentences a person would actually write. +// The same words, inside the code rather than the markup. +// +// `confirm()`, `alert()` and `app.toast()` take a string argument, and a string +// argument lives in the script block where the scanner above never looks. Six of +// them survived the whole i18n sweep that way — every destructive confirmation in +// the app, which is the worst possible set to leave in one language, because it is +// the text somebody reads in the half-second before they agree to lose something. +func TestNoDanishProseInDialogsAndToasts(t *testing.T) { + // The argument of one of the three, quoted or backticked. Deliberately narrow: + // a general scan of every string literal in a Svelte script block would drown + // in class names, keys and query fragments. + call := regexp.MustCompile(`(?:confirm|alert|app\.toast)\(\s*['` + "`" + `]([^'` + "`" + `]{4,})['` + "`" + `]`) + danish := regexp.MustCompile(`(?i)\b(og|er|ikke|det|den|der|til|kan|som|har|med|af|en|et|du|din|dine|vises|ingen|nyere|fundet|ude|valgt|slettet|gemt|opgave|projekt|indstillinger|fjern|opret|vælg|luk|tilføj|ryd|gem|slet|omdøb|bliver|holder|alle)\b`) + + var found []string + root := filepath.Join("..", "..", "web", "src") + err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if err != nil || info.IsDir() { + return err + } + if !strings.HasSuffix(path, ".svelte") && !strings.HasSuffix(path, ".js") { + return nil + } + if strings.Contains(path, "locales") { + return nil + } + body, err := os.ReadFile(path) + if err != nil { + return err + } + rel, _ := filepath.Rel(root, path) + for _, m := range call.FindAllStringSubmatch(string(body), -1) { + if danish.MatchString(m[1]) { + found = append(found, rel+": "+m[1]) + } + } + return nil + }) + if err != nil { + t.Fatalf("walk %s: %v", root, err) + } + + if len(found) > 0 { + t.Errorf("%d dialog or toast string(s) are written in Danish inside the code. "+ + "They are what somebody reads before agreeing to lose something, and an "+ + "English interface shows them in Danish:\n %s", + len(found), strings.Join(found, "\n ")) + } +} + func TestNoDanishProseOutsideTheDictionaries(t *testing.T) { // Text nodes of three characters or more, and the three attributes a person // reads. Anything inside {…} is already an expression — usually a t() call — diff --git a/web/src/lib/api.js b/web/src/lib/api.js index 36dfc2b..87280d5 100644 --- a/web/src/lib/api.js +++ b/web/src/lib/api.js @@ -55,6 +55,9 @@ const MESSAGES = { ai_not_configured: 'error.aiNotConfigured', totp_not_enabled: 'error.totpNotEnabled', inbox_protected: 'error.inboxProtected', + // The fallback for when Gmail refused and said nothing usable. When it did say + // something, `humanMessage` quotes it instead — see below. + gmail_failed: 'error.gmailRefused', last_admin: 'error.lastAdmin' }; @@ -67,6 +70,15 @@ const MESSAGES = { */ export function humanMessage(err) { if (!(err instanceof ApiError)) return t(MESSAGES.offline); + + // Gmail's own words are the diagnosis — "invalid_grant", "insufficient + // authentication scopes" — and a generic sentence in their place throws away + // the only thing that says what to do. Wrapped so it reads as a quotation + // rather than as a stray English string in a Danish interface. + if (err.code === 'gmail_failed' && err.message) { + return t('error.gmailSaid', { what: err.message }); + } + const key = MESSAGES[err.code]; if (key) return t(key); return err.message ?? t(MESSAGES.internal_error); diff --git a/web/src/lib/components/BoardView.svelte b/web/src/lib/components/BoardView.svelte index 53c215c..c62e587 100644 --- a/web/src/lib/components/BoardView.svelte +++ b/web/src/lib/components/BoardView.svelte @@ -125,7 +125,7 @@ ); } catch { app.replace(id, previous); - app.toast('Kunne ikke flytte opgaven.'); + app.toast(t('task.moveFailed')); } } diff --git a/web/src/lib/components/TaskDetail.svelte b/web/src/lib/components/TaskDetail.svelte index aaf710f..a41f347 100644 --- a/web/src/lib/components/TaskDetail.svelte +++ b/web/src/lib/components/TaskDetail.svelte @@ -276,7 +276,7 @@ } async function removeComment(comment) { - if (!confirm('Slet kommentaren?')) return; + if (!confirm(t('detail.deleteComment'))) return; const previous = comments; comments = comments.filter((c) => c.id !== comment.id); try { diff --git a/web/src/lib/locales/da.js b/web/src/lib/locales/da.js index e6352ad..0daef78 100644 --- a/web/src/lib/locales/da.js +++ b/web/src/lib/locales/da.js @@ -646,5 +646,24 @@ export const da = { 'update.waiting': 'Venter på at serveren kommer op igen…', 'update.stillDown': 'Serveren er ikke kommet op endnu. Kig i panelet.', 'update.notConfigured': - 'Denne instans kan ikke genstarte sig selv: {missing} er ikke sat. En container kan ikke udskifte sit eget image — panelet gør arbejdet, og det her er kun måden at bede om det.' + 'Denne instans kan ikke genstarte sig selv: {missing} er ikke sat. En container kan ikke udskifte sit eget image — panelet gør arbejdet, og det her er kun måden at bede om det.', + + 'error.gmailSaid': 'Gmail svarede: {what}', + + 'error.gmailRefused': 'Gmail afviste kaldet. Prøv at forbinde postkassen igen.', + + 'int.newAddressQuestion': 'Ny adresse? Mail til den gamle bliver afvist.', + 'int.copied': 'Kopieret.', + 'int.copyFailed': 'Kunne ikke kopiere — markér teksten i stedet.', + + 'detail.deleteComment': 'Slet kommentaren?', + 'int.disconnectQuestion': 'Afbryd forbindelsen til Gmail? Tokens bliver glemt.', + 'int.newLinkQuestion': 'Nyt feed-link? Alle eksisterende abonnementer holder op med at virke.', + 'data.deleteTemplate': 'Slet skabelonen "{name}"?', + 'tokens.revokeQuestion': 'Tilbagekald "{name}"? Alt, der bruger den, holder op med at virke.', + 'account.signOutThisDevice': 'Det er denne enhed. Du bliver logget ud.', + + 'project.deleteQuestion': 'Slet "{name}" med alle dets opgaver?', + 'project.deleteRecoverable': + 'Det havner i papirkurven og kan hentes tilbage under Indstillinger → Data og skabeloner i {days} dage.' }; diff --git a/web/src/lib/locales/en.js b/web/src/lib/locales/en.js index 21276b9..7dd3e94 100644 --- a/web/src/lib/locales/en.js +++ b/web/src/lib/locales/en.js @@ -641,5 +641,24 @@ export const en = { 'update.waiting': 'Waiting for the server to come back…', 'update.stillDown': 'The server has not come back yet. Have a look in the panel.', 'update.notConfigured': - 'This instance cannot restart itself: {missing} are not set. A container cannot replace its own image — the panel does the work, and this is only the way to ask.' + 'This instance cannot restart itself: {missing} are not set. A container cannot replace its own image — the panel does the work, and this is only the way to ask.', + + 'error.gmailSaid': 'Gmail said: {what}', + + 'error.gmailRefused': 'Gmail refused the call. Try connecting the mailbox again.', + + 'int.newAddressQuestion': 'A new address? Mail to the old one is refused.', + 'int.copied': 'Copied.', + 'int.copyFailed': 'Could not copy — select the text instead.', + + 'detail.deleteComment': 'Delete the comment?', + 'int.disconnectQuestion': 'Disconnect Gmail? The tokens are forgotten.', + 'int.newLinkQuestion': 'A new feed link? Every existing subscription stops working.', + 'data.deleteTemplate': 'Delete the template "{name}"?', + 'tokens.revokeQuestion': 'Revoke "{name}"? Everything using it stops working.', + 'account.signOutThisDevice': 'That is this device. You will be signed out.', + + 'project.deleteQuestion': 'Delete "{name}" with all its tasks?', + 'project.deleteRecoverable': + 'It goes to the trash and can be restored under Settings → Data and templates for {days} days.' }; diff --git a/web/src/routes/indstillinger/+page.svelte b/web/src/routes/indstillinger/+page.svelte index e8a3f7b..2b7c917 100644 --- a/web/src/routes/indstillinger/+page.svelte +++ b/web/src/routes/indstillinger/+page.svelte @@ -215,7 +215,7 @@ async function endSession(session) { if (session.current) { - if (!confirm('Det er denne enhed. Du bliver logget ud.')) return; + if (!confirm(t('account.signOutThisDevice'))) return; await api.endSession(session.id); // A full reload rather than clearing the store by hand: the session is // gone, so every open socket and every cached answer in this tab is now diff --git a/web/src/routes/indstillinger/data/+page.svelte b/web/src/routes/indstillinger/data/+page.svelte index b3b0034..03adf8b 100644 --- a/web/src/routes/indstillinger/data/+page.svelte +++ b/web/src/routes/indstillinger/data/+page.svelte @@ -138,7 +138,7 @@ } async function deleteTemplate(template) { - if (!confirm(`Slet skabelonen "${template.name}"?`)) return; + if (!confirm(t('data.deleteTemplate', { name: template.name }))) return; const previous = templates; templates = templates.filter((t) => t.id !== template.id); diff --git a/web/src/routes/indstillinger/integrationer/+page.svelte b/web/src/routes/indstillinger/integrationer/+page.svelte index 40ee888..6f02e9c 100644 --- a/web/src/routes/indstillinger/integrationer/+page.svelte +++ b/web/src/routes/indstillinger/integrationer/+page.svelte @@ -125,7 +125,7 @@ } async function disconnectGmail() { - if (!confirm('Afbryd forbindelsen til Gmail? Tokens bliver glemt.')) return; + if (!confirm(t('int.disconnectQuestion'))) return; try { await api.disconnectGmail(); gmail = await api.getGmail(); @@ -153,7 +153,7 @@ }); async function rotateFeed() { - if (!confirm('Nyt feed-link? Alle eksisterende abonnementer holder op med at virke.')) return; + if (!confirm(t('int.newLinkQuestion'))) return; try { feedURL = (await api.rotateFeed()).url; } catch (e) { @@ -162,7 +162,7 @@ } async function rotateMail() { - if (!confirm('Ny adresse? Mail til den gamle bliver afvist.')) return; + if (!confirm(t('int.newAddressQuestion'))) return; try { const r = await api.rotateMailAddress(); mailAddress = r.address; @@ -175,9 +175,9 @@ async function copy(text) { try { await navigator.clipboard.writeText(text); - app.toast('Kopieret.'); + app.toast(t('int.copied')); } catch { - app.toast('Kunne ikke kopiere — markér teksten i stedet.'); + app.toast(t('int.copyFailed')); } } diff --git a/web/src/routes/indstillinger/tokens/+page.svelte b/web/src/routes/indstillinger/tokens/+page.svelte index 99d09c7..1479194 100644 --- a/web/src/routes/indstillinger/tokens/+page.svelte +++ b/web/src/routes/indstillinger/tokens/+page.svelte @@ -60,7 +60,7 @@ } catch { // Insecure context, or permission refused. The token is on screen and // selectable, so this is a convenience that failed, not the feature. - app.toast('Kunne ikke kopiere — markér teksten i stedet.'); + app.toast(t('int.copyFailed')); } } @@ -69,12 +69,12 @@ await navigator.clipboard.writeText(connectorURL); connectorCopied = true; } catch { - app.toast('Kunne ikke kopiere — markér teksten i stedet.'); + app.toast(t('int.copyFailed')); } } async function revoke(token) { - if (!confirm(`Tilbagekald "${token.name}"? Alt, der bruger den, holder op med at virke.`)) + if (!confirm(t('tokens.revokeQuestion', { name: token.name }))) return; const previous = tokens; diff --git a/web/src/routes/projekt/[id]/+page.svelte b/web/src/routes/projekt/[id]/+page.svelte index 676e3cc..c838ae8 100644 --- a/web/src/routes/projekt/[id]/+page.svelte +++ b/web/src/routes/projekt/[id]/+page.svelte @@ -188,9 +188,9 @@ async function remove() { const days = 30; const confirmed = confirm( - `Slet "${project.name}" med alle dets opgaver?\n\n` + - `Det havner i papirkurven og kan hentes tilbage under ` + - `Indstillinger → Data og skabeloner i ${days} dage.` + t('project.deleteQuestion', { name: projectName(project) }) + + '\n\n' + + t('project.deleteRecoverable', { days }) ); if (!confirmed) return;