feat: never render an empty IMAP error detail and close the CRM rail when the viewport narrows past lg

This commit is contained in:
Matthew Meszaros
2026-09-09 09:24:43 -07:00
parent 73d9c18bfe
commit 568bdb48ba
3 changed files with 104 additions and 13 deletions
+20 -8
View File
@@ -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"
}
+64
View File
@@ -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)
}
}
}
+20 -5
View File
@@ -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.