(wip/draft) refactor: convert from snafu to n0-error #3561

Frando · 2025-10-20T10:33:56Z

Description

Note: 90% of this PR are written by an AI (OpenAI codex) which I instructed in numerous prompts to convert iroh from n0-snafu/snafu to n0-error. I told it to keep context in error-reporting.agents.md, which I added to the PR (will be removed before merge) for easy alteration.*

Note: The following description is also AI generated.

Migrate `snafu`/`n0-snafu` → `n0_error` across iroh workspace

Rationale

Unify error handling on n0_error (meta, display, source chaining). Remove per-crate snafu/n0-snafu. Consistent ergonomics across libs, bins, and examples.

Highlights

Replace snafu/n0-snafu with n0_error in: iroh, iroh-relay, iroh-base, iroh-dns-server, benches, examples, docs.
Conversions: use .std_context(..) for std/3rd-party errors; .context(..) only for local StackError types; use .e() in examples/tests where needed for AnyError.
Macros: snafu::whatever! → n0_error::whatever!; ensure_whatever! → explicit if !cond { whatever!(..) }.
add_meta/derive(Error): prefer #[error(from_sources, std_sources)] when feasible.

Gotchas

External errors (e.g., JoinError, WriteError, ReadToEndError, ClosedStream) don’t implement StackError in examples; use .e()? or .std_context(..).
Prefer bare ? for local StackError results; remove unnecessary .e() on internal Result<_>; re-add .e() only where compilation requires AnyError.

Review focus

Correct use of .context(..) vs .std_context(..).

Breaking Changes

Notes & open questions

Change checklist

github-actions · 2025-10-20T10:36:07Z

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3561/docs/iroh/

Last updated: 2025-10-21T07:49:51Z

github-actions · 2025-10-20T22:06:43Z

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 298d409

dignifiedquire · 2025-10-23T10:28:50Z

iroh-base/src/key.rs

        Ok(len) => {
            if len != PublicKey::LENGTH {
-                return Err(DecodeInvalidLengthSnafu.build());
+                return Err!(KeyParsingError::DecodeInvalidLength);


looking at this, I would much rather write

bail!(KeyParsingError::DecodeInvalidLength);

overloading Err I find problematic personally

Yes, I can add such a bail! macro. Without bail! or theErr! macro it would be
Err(e!(KeyParsingError::DecodeInvalidLength))

I think that would be better to have than Err!

dignifiedquire · 2025-10-23T10:29:29Z

iroh-base/src/key.rs

        self.as_verifying_key()
            .verify_strict(message, &signature.0)
-            .map_err(|_| SignatureSnafu.build())
+            .map_err(|_| e!(SignatureError))


does this have to be a macro, or could this be a method on SignatureError?

Right, this is a struct so it can just be SignatureError::new() in this case. However, for enums I removed the constructors from the proc macro generated code (I had them before) so that jump-to-definition on variants works.

dignifiedquire · 2025-10-23T10:30:39Z

iroh-base/src/ticket.rs

        let expected = Self::KIND;
        let Some(rest) = str.strip_prefix(expected) else {
-            return Err(KindSnafu { expected }.build());
+            return Err(ParseError::wrong_prefix(expected));


shouldn't this be called ParseError::kind(expected)?

nvm, found the impl below

could we have a macro so we can write

bail!(ParseError::Kind { expected });

without the indirection method?

Yeah, this was just kept from existing code. It would either be bail or return Err!(ParseError::Kind { expected }) or return Err(e!(ParseError::Kind { expected })). Which one we want is to be bikeshedded.

dignifiedquire · 2025-10-23T10:33:10Z

iroh-dns-server/src/config.rs

        } else {
            let path = dirs_next::data_dir()
-                .context("operating environment provides no directory for application data")?;
+                .std_context("operating environment provides no directory for application data")?;


I think this works reasonably well, a bit confusing at first, but easy to use at the end of the day

To me, this is the most annoying thing about this whole endevour, but unfortunately I don't think there's another solution. I made the std_context method the "longer" one (to type) to nudge towards using context wherever possible (ie. on our errors that implement StackError) because std_context will work on our errors too, but we'll lose the location info in the process. This is the much discussed issue that as long as we implement std::error::Error for our errors, we can't forbid calling the StdResultExt methods on them, even though we should always use the StackResultExt methods (which are only available on StackError results, not on std error results).

dignifiedquire · 2025-10-23T10:34:08Z

iroh-dns-server/src/http.rs

                Ok(Err(err)) => {
                    warn!(?err, "task failed");
-                    final_res = Err(err).context("task");
+                    final_res = Err(err).std_context("task");


could we have something like

final_res = whatever_std!(err, "task");

We can. Here it even can be the same macro for both StackError and std error, because we can use autoref specialization.

There is already a anyerr! macro that makes use of autoref specialization to not have to worry about whether the error is std or StackError. With this it would be Err(anyerr!(err).context("task")). I can expand the macro to support context as second arg, so it could be Err(anyerr!(err, "task")). Will do that.
Not sure if we need a variant that returns Err(err), I think it's fine to write that by hand (like anyhow::anyhow!, it also returns the error not a result).

dignifiedquire · 2025-10-23T10:37:49Z

iroh-relay/src/client.rs

-                    url: dial_url.clone(),
-                }
-                .build()
+                e!(ConnectError::InvalidWebsocketUrl {


this macro fills in the location details?

Yes. It expands to ConnectError::InvalidWebsocketUrl { url: dial_url.clone(), meta: Default::default() }

dignifiedquire · 2025-10-23T10:38:59Z

iroh-relay/src/client/conn.rs

-        snafu::ensure!(size <= MAX_PACKET_SIZE, ExceedsMaxPacketSizeSnafu { size });
+        n0_error::ensure!(
+            size <= MAX_PACKET_SIZE,
+            n0_error::e!(SendError::ExceedsMaxPacketSize { size })


could we make the macro such that we don't need to call e! inside?

We can but then it wouldn't be usable with other error types anymore as it would add a meta field and if that doesn't exist it would give a compile error. We could add another macro ensure_e! or such, but not sure if that is really better than the current one. Will change to import e though, then it's much shorter already.

dignifiedquire · 2025-10-23T10:40:09Z

iroh-relay/src/client/tls.rs

        })
-        .await??;
+        .await
+        .map_err(|err| e!(DialError::Timeout, err))??;


what does this variant do?

Expand to DialError::Timeout { source: err, meta: Meta::default() }
See docs: https://n0-computer.github.io/n0-error/pr/4/docs/n0_error/macro.e.html

dignifiedquire · 2025-10-23T10:41:58Z

iroh-relay/src/client/tls.rs

-        let upgraded = hyper::upgrade::on(res).await.context(ProxyConnectSnafu)?;
+        let upgraded = hyper::upgrade::on(res)
+            .await
+            .map_err(|err| e!(DialError::ProxyConnect, err))?;


can't we do this with one of the context variants?

context takes a string (or, impl Display) and adds this as context. We can't do without the e! macro if we want jump-to-usage on ProxyConnect to work and not type meta: Default::default() manually. So not sure what context should do here?

yeah, the filling in the struct wouldn't be possible :(

dignifiedquire · 2025-10-23T10:42:50Z

iroh-relay/src/dns.rs

+    errors: Vec<LookupError>,
 }

+// no manual new; use e!(Type { errors })


random comment

will remove

Frando · 2025-10-23T11:32:24Z

Some examples when tests are failing:

Error: Unable to connect to remote (at iroh/src/endpoint.rs:633:26)
Caused by:
    0: Unable to connect to remote (at iroh/src/endpoint.rs:735:28)
    1: endpoint stopping

Here we get the same message twice, because the outer error delegates to an inner error with #[transparent].

Failed to create bind sockets (at iroh/src/endpoint.rs:202:21)
Caused by:
    0: Failed to create bind sockets (at iroh/src/magicsock.rs:1389:31)
    1: failed to bind

Same: the outer error delegates to the next with transparent, so same message twice.

Another one:

Error: No addressing information available (at iroh/src/endpoint.rs:699:28)
Caused by:
    0: Discovery service failed (at iroh/src/endpoint.rs:1291:39)
    1: Discovery produced no results for b371e8d4c1 (at iroh/src/discovery.rs:645:21)

Frando mentioned this pull request Oct 20, 2025

(draft) use n0-error #3511

Closed

11 tasks

n0bot bot added this to iroh Oct 20, 2025

github-project-automation bot moved this to 🏗 In progress in iroh Oct 20, 2025

Frando added 18 commits October 20, 2025 23:54

refactor: convert iroh-base to n0-error

5552d9a

fixup

93fb21c

refactor: convert iroh-relay

82f4bbd

wip

0b4c014

wip: converting iroh

9650632

fmt

54fef34

fixup

39a5935

fixup

3cd15f9

fixup

f4ec732

codex is converting iroh

923694c

fixes

9d39c78

wip

4e830d1

codex did it

894ff10

fix & fmt

47064a3

chore: clippy fix & cargo fix & fmt

8afe0b7

fixup

4365e91

fixup

099e2b0

fixup

7f59c66

Frando force-pushed the Frando/n0-error branch from 32458a9 to 7f59c66 Compare October 20, 2025 22:00

Frando added 6 commits October 21, 2025 09:15

fixup

587c5c6

fixup

35fa204

fixup

383ed04

fixup

7e9b997

fixup

6969f68

fixup

ea9b541

dignifiedquire reviewed Oct 23, 2025

View reviewed changes

(wip/draft) refactor: convert from snafu to n0-error #3561

Are you sure you want to change the base?

(wip/draft) refactor: convert from snafu to n0-error #3561

Uh oh!

Conversation

Frando commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Migrate snafu/n0-snafu → n0_error across iroh workspace

Rationale

Highlights

Gotchas

Review focus

Breaking Changes

Notes & open questions

Change checklist

Uh oh!

github-actions bot commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Frando Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Frando commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Frando commented Oct 20, 2025 •

edited

Loading

Migrate `snafu`/`n0-snafu` → `n0_error` across iroh workspace

github-actions bot commented Oct 20, 2025 •

edited

Loading

github-actions bot commented Oct 20, 2025 •

edited

Loading

Frando Oct 23, 2025 •

edited

Loading

Frando commented Oct 23, 2025 •

edited

Loading