Collect all the parse errors during Walk and only expicitly fail, when
the callback errs. Return the Joined errors along with the file names.

Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com>
---
 maildir.go | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/maildir.go b/maildir.go
index dc3d9d2..d537405 100644
--- a/maildir.go
+++ b/maildir.go
@@ -82,7 +82,6 @@ func (d Dir) Unseen() ([]string, error) {
 		} else if err != nil {
 			return keys, err
 		}
-
 		for _, n := range names {
 			if n[0] == '.' {
 				continue
@@ -168,6 +167,7 @@ func (d Dir) Walk(fn func(key string, flags []Flag) error) error {
 	}
 	defer f.Close()
 
+	var errs error
 	for {
 		names, err := f.Readdirnames(readdirChunk)
 		if errors.Is(err, io.EOF) {
@@ -183,21 +183,24 @@ func (d Dir) Walk(fn func(key string, flags []Flag) error) error {
 
 			key, err := parseKey(n)
 			if err != nil {
-				return err
+				errs = errors.Join(errs, fmt.Errorf("%s: %w", n, err))
+				continue
 			}
 
 			flags, err := parseFlags(n)
 			if err != nil {
-				return err
+				errs = errors.Join(errs, fmt.Errorf("%s: %w", n, err))
+				continue
 			}
 
 			if err := fn(key, flags); err != nil {
-				return err
+				errs = errors.Join(errs, fmt.Errorf("%s: %w", n, err))
+				return errs
 			}
 		}
 	}
 
-	return nil
+	return errs
 }
 
 // Keys returns a slice of valid keys to access messages by.
-- 
2.43.0

Hey,

On Sun Jan 28, 2024 at 13:58, Bence Ferdinandy <bence@ferdinandy.com> wrote:
> Collect all the parse errors during Walk and only expicitly fail, when
> the callback errs. Return the Joined errors along with the file names.
>
> Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com>
> ---

just pinging this, to see if you have any thought. As a reminder, it came up in
this issue: https://github.com/emersion/go-maildir/issues/23

Thanks,
Bence

Thanks for the ping, and sorry for the delay.

When you sent this patch, I've reworked it a bit [1] so that we don't
create long chains of nested errors (instead a single errors.Join),
fix a test failure and drop the error prefix (already included in the
existing errors).

The reason I haven't pushed this is that I'm still wondering how to
handle errors from the user-provided callback. Right now callers expect
that an error returned from the callback will be passed as-is, but with
this patch it's no longer the case. I don't want to special-case the
situation where there is no format error but there is a callback error,
because this would result in unreliable error handling in the callers
(it would appear to work if there is no format failure, but would break
as soon as there is one).

Probably the way to go is to always wrap the error with errors.Join as
done in the branch, and document this as a breaking change?

[1]: https://github.com/emersion/go-maildir/compare/master...walk-err

On Tue Apr 30, 2024 at 11:50, Simon Ser <contact@emersion.fr> wrote:
> Thanks for the ping, and sorry for the delay.

No worries, I wasn't exactly super speedy in getting back here myself ^^

>
> When you sent this patch, I've reworked it a bit [1] so that we don't
> create long chains of nested errors (instead a single errors.Join),
> fix a test failure and drop the error prefix (already included in the
> existing errors).

That indeed looks better!

>
> The reason I haven't pushed this is that I'm still wondering how to
> handle errors from the user-provided callback. Right now callers expect
> that an error returned from the callback will be passed as-is, but with
> this patch it's no longer the case. I don't want to special-case the
> situation where there is no format error but there is a callback error,
> because this would result in unreliable error handling in the callers
> (it would appear to work if there is no format failure, but would break
> as soon as there is one).
>
> Probably the way to go is to always wrap the error with errors.Join as
> done in the branch, and document this as a breaking change?

I think yes, you are right, this is probably the best solution here. Users can
still parse the returned error themselves, so it shouldn't be a big deal to
update any downstream code.

Thanks,
Bence

I've added some docs and pushed the patch.

On Sat Jul 06, 2024 at 16:06, Simon Ser <contact@emersion.fr> wrote:
> I've added some docs and pushed the patch.

Great, thanks! Could I also convince you to tag a release for this? Unless of
course you see one coming soonish anyway. I could then patch aerc to use this.

Thanks,
Bence

On Saturday, July 6th, 2024 at 21:49, Bence Ferdinandy <bence@ferdinandy.com> wrote:

> On Sat Jul 06, 2024 at 16:06, Simon Ser contact@emersion.fr wrote:
> 
> > I've added some docs and pushed the patch.
> 
> Great, thanks! Could I also convince you to tag a release for this? Unless of
> course you see one coming soonish anyway. I could then patch aerc to use this.

Yes, done:
https://github.com/emersion/go-maildir/releases/tag/v0.5.0

On Wed Jul 10, 2024 at 01:23, Simon Ser <contact@emersion.fr> wrote:
> On Saturday, July 6th, 2024 at 21:49, Bence Ferdinandy <bence@ferdinandy.com> wrote:
>
> > On Sat Jul 06, 2024 at 16:06, Simon Ser contact@emersion.fr wrote:
> > 
>
> > > I've added some docs and pushed the patch.
> > 
>
> > Great, thanks! Could I also convince you to tag a release for this? Unless of
> > course you see one coming soonish anyway. I could then patch aerc to use this.
>
> Yes, done:
> https://github.com/emersion/go-maildir/releases/tag/v0.5.0

Awesome, thanks!





-- 
+36305425054
bence.ferdinandy.com