Skip to content

fix(sender): add SMTP dial timeouts#1692

Open
kawacukennedy wants to merge 4 commits into
floatpane:masterfrom
kawacukennedy:fix/smtp-dial-timeout
Open

fix(sender): add SMTP dial timeouts#1692
kawacukennedy wants to merge 4 commits into
floatpane:masterfrom
kawacukennedy:fix/smtp-dial-timeout

Conversation

@kawacukennedy

@kawacukennedy kawacukennedy commented Jul 20, 2026

Copy link
Copy Markdown

What?

Add 30-second connection timeouts to both SMTP dial paths (implicit TLS on port 465 and plain TCP for other ports).

Why?

Both tls.Dial and smtp.Dial blocked indefinitely if the remote server was slow to respond, a firewall silently dropped packets, or DNS resolution stalled. This hung the entire application with no way to cancel.

Also fixed IPv6 address formatting (fmt.Sprintfnet.JoinHostPort).

@kawacukennedy
kawacukennedy requested a review from a team as a code owner July 20, 2026 00:59
@floatpanebot floatpanebot added area/sender SMTP send path size/S Diff: 11–50 lines and removed area/sender SMTP send path labels Jul 20, 2026
Both tls.Dial (port 465) and smtp.Dial (other ports) could block
indefinitely if the remote server is slow to respond, a firewall
silently drops packets, or DNS resolution is slow. This caused the
entire application to hang on outgoing mail.

Replace with tls.DialWithDialer and net.DialTimeout + smtp.NewClient,
both using a 30-second timeout. Also switch addr formatting from
fmt.Sprintf to net.JoinHostPort for correct IPv6 literal handling.
@kawacukennedy
kawacukennedy force-pushed the fix/smtp-dial-timeout branch from 4bfb770 to e545e99 Compare July 20, 2026 01:30
@floatpanebot floatpanebot added area/sender SMTP send path size/XL Diff: 800+ lines labels Jul 20, 2026
@floatpanebot

Copy link
Copy Markdown
Member

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@floatpanebot floatpanebot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kawacukennedy! Please fix the following issues with your PR:

  • Title: Is too long (49 characters). The PR title must be strictly under 40 characters.
  • Body: Missing the ## What? or ## Why? headings required by the PR template.

@floatpanebot

floatpanebot commented Jul 20, 2026

Copy link
Copy Markdown
Member

Benchmark report — no significant change

Metrics worse: 0 · better: 0 (threshold: ±3%).

benchstat output
goos: linux
goarch: amd64
pkg: github.com/floatpane/matcha/backend
cpu: AMD EPYC 7763 64-Core Processor                
                           │    old.txt    │               new.txt               │
                           │    sec/op     │    sec/op      vs base              │
ParseSearchQuery_Simple-4    2.846µ ±  22%   2.999µ ± 330%       ~ (p=0.485 n=6)
ParseSearchQuery_Complex-4   8.030µ ± 136%   8.306µ ±  65%       ~ (p=0.937 n=6)
TokenizeSearchQuery-4        9.783µ ±  66%   9.860µ ± 141%       ~ (p=1.000 n=6)
geomean                      6.069µ          6.262µ         +3.19%

                           │  old.txt   │              new.txt               │
                           │    B/op    │    B/op     vs base                │
ParseSearchQuery_Simple-4    26.00 ± 0%   26.00 ± 0%       ~ (p=1.000 n=6) ¹
ParseSearchQuery_Complex-4   762.0 ± 0%   762.0 ± 0%       ~ (p=1.000 n=6) ¹
TokenizeSearchQuery-4        176.0 ± 0%   176.0 ± 0%       ~ (p=1.000 n=6) ¹
geomean                      151.6        151.6       +0.00%
¹ all samples are equal

                           │  old.txt   │              new.txt               │
                           │ allocs/op  │ allocs/op   vs base                │
ParseSearchQuery_Simple-4    2.000 ± 0%   2.000 ± 0%       ~ (p=1.000 n=6) ¹
ParseSearchQuery_Complex-4   23.00 ± 0%   23.00 ± 0%       ~ (p=1.000 n=6) ¹
TokenizeSearchQuery-4        9.000 ± 0%   9.000 ± 0%       ~ (p=1.000 n=6) ¹
geomean                      7.453        7.453       +0.00%
¹ all samples are equal

pkg: github.com/floatpane/matcha/tui
                    │   old.txt    │              new.txt               │
                    │    sec/op    │    sec/op     vs base              │
LogPanelView-4        176.5µ ± 12%   173.2µ ± 18%       ~ (p=1.000 n=6)
SearchOverlayView-4   189.4µ ±  5%   192.7µ ± 10%       ~ (p=0.589 n=6)
InboxConstruction-4   1.036m ±  9%   1.029m ± 16%       ~ (p=0.699 n=6)
geomean               325.9µ         325.1µ        -0.25%

                    │   old.txt    │               new.txt               │
                    │     B/op     │     B/op       vs base              │
LogPanelView-4        44.67Ki ± 0%   44.67Ki ± 51%       ~ (p=1.000 n=6)
SearchOverlayView-4   56.15Ki ± 0%   56.15Ki ± 41%       ~ (p=0.859 n=6)
InboxConstruction-4   874.3Ki ± 0%   874.4Ki ±  0%       ~ (p=0.485 n=6)
geomean               129.9Ki        129.9Ki        +0.00%

                    │   old.txt   │              new.txt              │
                    │  allocs/op  │  allocs/op   vs base              │
LogPanelView-4         714.0 ± 0%    714.0 ± 0%       ~ (p=1.000 n=6)
SearchOverlayView-4    926.0 ± 0%    926.0 ± 0%       ~ (p=0.545 n=6)
InboxConstruction-4   3.478k ± 0%   3.478k ± 0%       ~ (p=0.974 n=6)
geomean               1.320k        1.320k       +0.00%

auto-generated by benchmarks.yml

@kawacukennedy kawacukennedy changed the title fix(sender): add connection timeouts to SMTP dial fix(sender): add SMTP dial timeouts Jul 20, 2026
@floatpanebot
floatpanebot dismissed their stale review July 20, 2026 01:33

Formatting issues have been resolved. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sender SMTP send path size/S Diff: 11–50 lines size/XL Diff: 800+ lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants