chore(daemon): named timeout constants (#1027)

Matt Van Horn and Matt Van Horn created 2 months ago

## What?

Replaces ad-hoc `60*time.Second` and `30*time.Second` literals in
`daemon/handler.go` with two named constants and a brief comment
documenting the split, exactly as suggested in the issue body.

```go
const (
    fetchTimeout  = 60 * time.Second
    mutateTimeout = 30 * time.Second
)
```

`fetchTimeout` (was `60*time.Second`):
- `handleFetchEmails` (line 129)
- `handleFetchEmailBody` (line 154)
- async fetch inside `handleMarkRead` (line 318)

`mutateTimeout` (was `30*time.Second`):
- `handleDeleteEmails` (line 193)
- `handleArchiveEmails` (line 216)
- `handleMoveEmails` (line 239)
- `handleMarkRead` (line 262)
- `handleFetchFolders` (line 287)

## Why?

> `fetchTimeout` covers reads against the upstream IMAP provider, which
can return large bodies and so are given more headroom. `mutateTimeout`
covers state-changing operations and folder listings, which are bounded
by IMAP command latency rather than payload size.

If the maintainer wants `handleFetchFolders` reclassified to
`fetchTimeout` (which would be a behavior change from 30s to 60s), happy
to do that as a follow-up — kept it at the existing value here to keep
this purely a refactor.

Closes #985.

Co-authored-by: Matt Van Horn <[e-mail]>

Change summary

daemon/handler.go | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)

Detailed changes

daemon/handler.go

@@ -11,6 +11,15 @@ import (
 	"github.com/floatpane/matcha/daemonrpc"
 )
 
+// Per-handler timeouts. fetchTimeout covers reads against the upstream IMAP
+// provider, which can return large bodies and so are given more headroom.
+// mutateTimeout covers state-changing operations and folder listings, which
+// are bounded by IMAP command latency rather than payload size.
+const (
+	fetchTimeout  = 60 * time.Second
+	mutateTimeout = 30 * time.Second
+)
+
 func (d *Daemon) handleRequest(conn *daemonrpc.Conn, req *daemonrpc.Request) {
 	switch req.Method {
 	case daemonrpc.MethodPing:
@@ -117,7 +126,7 @@ func (d *Daemon) handleFetchEmails(conn *daemonrpc.Conn, req *daemonrpc.Request)
 		return
 	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
+	ctx, cancel := context.WithTimeout(context.Background(), fetchTimeout)
 	defer cancel()
 
 	emails, err := p.FetchEmails(ctx, params.Folder, params.Limit, params.Offset)
@@ -142,7 +151,7 @@ func (d *Daemon) handleFetchEmailBody(conn *daemonrpc.Conn, req *daemonrpc.Reque
 		return
 	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
+	ctx, cancel := context.WithTimeout(context.Background(), fetchTimeout)
 	defer cancel()
 
 	body, attachments, err := p.FetchEmailBody(ctx, params.Folder, params.UID)
@@ -181,7 +190,7 @@ func (d *Daemon) handleDeleteEmails(conn *daemonrpc.Conn, req *daemonrpc.Request
 		return
 	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	ctx, cancel := context.WithTimeout(context.Background(), mutateTimeout)
 	defer cancel()
 
 	if err := p.DeleteEmails(ctx, params.Folder, params.UIDs); err != nil {
@@ -204,7 +213,7 @@ func (d *Daemon) handleArchiveEmails(conn *daemonrpc.Conn, req *daemonrpc.Reques
 		return
 	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	ctx, cancel := context.WithTimeout(context.Background(), mutateTimeout)
 	defer cancel()
 
 	if err := p.ArchiveEmails(ctx, params.Folder, params.UIDs); err != nil {
@@ -227,7 +236,7 @@ func (d *Daemon) handleMoveEmails(conn *daemonrpc.Conn, req *daemonrpc.Request)
 		return
 	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	ctx, cancel := context.WithTimeout(context.Background(), mutateTimeout)
 	defer cancel()
 
 	if err := p.MoveEmails(ctx, params.UIDs, params.SourceFolder, params.DestFolder); err != nil {
@@ -250,7 +259,7 @@ func (d *Daemon) handleMarkRead(conn *daemonrpc.Conn, req *daemonrpc.Request) {
 		return
 	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	ctx, cancel := context.WithTimeout(context.Background(), mutateTimeout)
 	defer cancel()
 
 	// MarkAsRead only supports one UID at a time in the Provider interface.
@@ -275,7 +284,7 @@ func (d *Daemon) handleFetchFolders(conn *daemonrpc.Conn, req *daemonrpc.Request
 		return
 	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	ctx, cancel := context.WithTimeout(context.Background(), mutateTimeout)
 	defer cancel()
 
 	folders, err := p.FetchFolders(ctx)
@@ -306,7 +315,7 @@ func (d *Daemon) handleRefreshFolder(conn *daemonrpc.Conn, req *daemonrpc.Reques
 			Folder:    params.Folder,
 		})
 
-		ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
+		ctx, cancel := context.WithTimeout(context.Background(), fetchTimeout)
 		defer cancel()
 
 		emails, err := p.FetchEmails(ctx, params.Folder, 50, 0)