Skip to content

fix(pop3): wrap underlying errors#1320

Merged
andrinoff merged 1 commit into
floatpane:masterfrom
mvanhorn:osc/688-pop3-error-wrapping
May 21, 2026
Merged

fix(pop3): wrap underlying errors#1320
andrinoff merged 1 commit into
floatpane:masterfrom
mvanhorn:osc/688-pop3-error-wrapping

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

What?

findAttachmentData in backend/pop3/pop3.go had three error sites that dropped underlying context:

  • gomail.CreateReader returned an error but the wrapping call discarded it with a static "not a multipart message" string.
  • The mr.NextPart() loop broke on a non-EOF error without saving it, so the final return at the bottom of the function reported "not found" instead of the real read failure.
  • The not-found error had no count of how many parts the loop actually scanned, which made the failure hard to debug from a log line alone.

This PR wraps the CreateReader error with %w, captures the NextPart error in a local scanErr and surfaces it as the final error when it is set, and includes the scanned-parts count in the not-found message.

Why?

Fixes #688. The reporter noted the missing %w wrapping at the original line 438. While poking at that site I noticed the silent break on mr.NextPart() errors right above it. Those two paths share the same downstream "not found" return, so a multipart parse error in the middle of the stream looks identical to a genuinely missing part. The third change (scanned-parts count) is the smallest possible nudge that makes the two failure modes distinguishable in an error string without changing the return type or call sites.

The change is local to findAttachmentData and does not touch any caller. Existing tests in backend/pop3/pop3_test.go still pass; go build, go vet, and gofmt -l are clean.

Fixes #688

AI-assisted.

Three error sites in findAttachmentData drop underlying error context:
- gomail.CreateReader error is replaced by a static "not a multipart
  message" message
- mr.NextPart() non-EOF errors are silently swallowed by the loop
- the not-found error has no scanned-parts count

Wrap the CreateReader error with %w, capture the scan error and surface it
as the final error when present, and include the scanned-parts count in
the not-found message.

Fixes floatpane#688
@mvanhorn mvanhorn requested a review from a team as a code owner May 21, 2026 06:50
@floatpanebot floatpanebot added bug Something isn't working ci CI / build pipeline labels May 21, 2026
Copy link
Copy Markdown
Member

@floatpanebot floatpanebot left a comment

Choose a reason for hiding this comment

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

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

  • Title: Is too long (55 characters). The PR title must be strictly under 40 characters.

@floatpanebot floatpanebot added the size/S Diff: 11–50 lines label May 21, 2026
@mvanhorn mvanhorn changed the title fix(pop3): wrap underlying errors in attachment parsing fix(pop3): wrap underlying errors May 21, 2026
@floatpanebot floatpanebot dismissed their stale review May 21, 2026 09:11

Formatting issues have been resolved. Thank you!

Copy link
Copy Markdown
Member

@FromSi FromSi left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Copy Markdown
Member

@andrinoff andrinoff left a comment

Choose a reason for hiding this comment

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

lgtm

@andrinoff andrinoff merged commit cfbb604 into floatpane:master May 21, 2026
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ci CI / build pipeline size/S Diff: 11–50 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: Missing error wrapping in pop3 attachment parsing

4 participants