diff --git a/internal/client/smtpimap/imap/err.go b/internal/client/smtpimap/imap/err.go index 53c0aaa3..e4e41ab2 100644 --- a/internal/client/smtpimap/imap/err.go +++ b/internal/client/smtpimap/imap/err.go @@ -2,6 +2,8 @@ package imap import ( "errors" + "fmt" + "strings" "github.com/emersion/go-imap/v2" "github.com/warmbly/warmbly/internal/errx" @@ -21,14 +23,7 @@ func (c *Client) handleError(err error) *errx.MailError { case imap.ResponseCodeAuthorizationFailed: return errx.ErrMailAuthorizationFailed default: - // A NO/BAD without a response code (Gmail's "NO System Error") - // has only its text; an empty code rendered as "Something went - // wrong: " in the mailbox's error list. - detail := string(imapErr.Code) - if detail == "" { - detail = imapErr.Text - } - return errx.ErrMailUnknownImapError(detail) + return errx.ErrMailUnknownImapError(imapErrDetail(imapErr)) } } @@ -44,3 +39,20 @@ func (c *Client) handleError(err error) *errx.MailError { // pass instead of deactivating the mailbox. return errx.ErrMailServerUnreachable } + +// imapErrDetail is the part of the mailbox's error row that says what the +// server actually refused. The response code is optional in IMAP, and a +// codeless NO/BAD (IONOS, Gmail's "NO System Error") rendered as "Something +// went wrong: " with nothing after the colon, which is unactionable for the +// customer and undiagnosable from a bug report. Never returns "". +func imapErrDetail(err *imap.Error) string { + if err.Code != "" { + return string(err.Code) + } + // Keep the status: a BAD means we sent something the server does not + // understand, a NO means it understood and declined. + if detail := strings.TrimSpace(fmt.Sprintf("%s %s", err.Type, err.Text)); detail != "" { + return detail + } + return "the mail server refused the command without saying why" +} diff --git a/internal/client/smtpimap/imap/err_test.go b/internal/client/smtpimap/imap/err_test.go index 6eb719a4..694f46e8 100644 --- a/internal/client/smtpimap/imap/err_test.go +++ b/internal/client/smtpimap/imap/err_test.go @@ -3,8 +3,10 @@ package imap import ( "io" "net" + "strings" "testing" + goimap "github.com/emersion/go-imap/v2" "github.com/warmbly/warmbly/internal/errx" ) @@ -26,3 +28,65 @@ func TestHandleErrorTransportIsNotNil(t *testing.T) { t.Error("handleError(nil) must stay nil") } } + +// A NO/BAD carries a response code only when the server chooses to send one. +// A codeless one used to render as "Something went wrong: " with nothing after +// the colon (issue #405, IONOS), which tells the customer nothing and leaves a +// bug report with no way to identify the refused command. +func TestHandleErrorCodelessImapErrorKeepsServerText(t *testing.T) { + c := &Client{} + + for _, tc := range []struct { + name string + err *goimap.Error + want string + }{ + { + name: "codeless NO keeps the server's text", + err: &goimap.Error{Type: goimap.StatusResponseTypeNo, Text: "System Error"}, + want: "NO System Error", + }, + { + name: "codeless BAD is distinguishable from a NO", + err: &goimap.Error{Type: goimap.StatusResponseTypeBad, Text: "Command unrecognized"}, + want: "BAD Command unrecognized", + }, + { + name: "a response code still wins over the text", + err: &goimap.Error{Type: goimap.StatusResponseTypeNo, Code: goimap.ResponseCodeServerBug, Text: "oops"}, + want: "SERVERBUG", + }, + { + name: "no code and no text still says something", + err: &goimap.Error{Type: goimap.StatusResponseTypeNo}, + want: "NO", + }, + } { + t.Run(tc.name, func(t *testing.T) { + got := c.handleError(tc.err) + if got == nil { + t.Fatalf("handleError(%v) = nil, want a mail error", tc.err) + } + if got.Code != errx.MailErrorCodeImapUnknown { + t.Fatalf("Code = %q, want %q", got.Code, errx.MailErrorCodeImapUnknown) + } + if !strings.Contains(got.Message, tc.want) { + t.Errorf("Message = %q, want it to contain %q", got.Message, tc.want) + } + }) + } +} + +// The whole point of the fallback: the detail is never empty, so the row can +// never read as a bare "Something went wrong: " again. +func TestHandleErrorImapDetailIsNeverEmpty(t *testing.T) { + for _, err := range []*goimap.Error{ + {}, + {Type: goimap.StatusResponseTypeNo}, + {Type: goimap.StatusResponseTypeBad, Text: " "}, + } { + if detail := imapErrDetail(err); strings.TrimSpace(detail) == "" { + t.Errorf("imapErrDetail(%+v) = %q, want a non-empty detail", err, detail) + } + } +} diff --git a/web/src/components/app/unibox/ThreadView.tsx b/web/src/components/app/unibox/ThreadView.tsx index 8e55b361..641928b3 100644 --- a/web/src/components/app/unibox/ThreadView.tsx +++ b/web/src/components/app/unibox/ThreadView.tsx @@ -160,11 +160,26 @@ export function ThreadView({ threadId, emailId }: ThreadViewProps) { const threadLabels = useThreadLabels(threadId); const [labelMenuOpen, setLabelMenuOpen] = React.useState(false); - // CRM context rail (right side). Starts closed at every width and is - // opened from the header toggle: on wide screens it renders as a static - // rail, below lg as an overlay drawer. It used to open by itself on lg+, - // which put the contact form in front of every thread the reader opened. - const [crmOpen, setCrmOpen] = React.useState(false); + // CRM context rail (right side). Open by default on lg+, where it renders + // as a static rail beside the thread; below lg it is an overlay drawer, so + // it starts closed and is opened from the header toggle. + const [crmOpen, setCrmOpen] = React.useState( + () => + typeof window !== "undefined" && + window.matchMedia("(min-width: 1024px)").matches, + ); + + // The initial state is read once, so narrowing past lg with the rail open + // turned it into an overlay sitting on top of the thread (a rotated tablet, + // a window dragged to half a screen). Close it on the way down. + React.useEffect(() => { + const mq = window.matchMedia("(min-width: 1024px)"); + const onChange = (e: MediaQueryListEvent) => { + if (!e.matches) setCrmOpen(false); + }; + mq.addEventListener("change", onChange); + return () => mq.removeEventListener("change", onChange); + }, []); // `c` opens the label menu while a thread is open — ignored while // typing into the composer / any input so it never eats keystrokes.