Fix Result type generation for user-defined error enums named "Error"

[willemneal]

What

When a contract defines its own error enum named Error and uses it in a Result<T, Error> return type, the generated client code now emits Result<T, Error> (referencing the user-defined enum) instead of Result<T, soroban_sdk::Error>.

The fix lives entirely in client generation. The on-the-wire spec format is unchanged: a contract function returning Result<_, Error> still serializes the error type as ScSpecTypeDef::Error, exactly as every prior SDK release has done. During Rust client generation, soroban-spec-rust now checks whether the spec also contains a UdtErrorEnumV0 named Error, and if so rewrites every ScSpecTypeDef::Error reference to Udt { name: "Error" } so that the generated trait references the user-defined type.

If the spec contains no error enum named Error, behavior is unchanged: ScSpecTypeDef::Error continues to map to soroban_sdk::Error, which covers contracts that intentionally use the SDK error type at their boundary.

Why

Previously, contracts whose error enum was named Error (a natural choice — it's what every other Rust crate would name it) silently produced a generated client that returned Result<_, soroban_sdk::Error> instead of Result<_, Error>, hiding the contract's error variants behind the opaque SDK error type and breaking client-side error matching.

This iteration of the fix moves the resolution from spec emission to client interpretation, per review feedback (#1709 (comment)). Two benefits over the original approach of changing what #[contractimpl] emits:

1. No spec wire-format change. The bytes that go into a deployed wasm are identical to what main produces today, so no contract's spec hash drifts.
2. Already-deployed contracts benefit immediately. Any mainnet contract that defines an Error error enum will, the next time someone regenerates a Rust client against its spec with the new soroban-spec-rust, produce a correct Result<_, Error> signature — no redeploy required.

Example — before the fix:

// Contract defines:
pub enum Error { Overflow = 1 }
pub fn safe_add(a: u64, b: u64) -> Result<u64, Error>

// Generated client had:
fn try_safe_add(...) -> Result<..., Result<soroban_sdk::Error, ...>>

Example — after the fix:

// Generated client now correctly has:
fn try_safe_add(...) -> Result<..., Result<Error, ...>>

Closes #1710

Known limitations

If a contract both defines an error enum named Error and uses soroban_sdk::Error directly at a function boundary, every Error reference in the spec resolves to the user-defined UDT — there is no way to mix the two in one spec. This matches user intent: a contract that names its own error enum Error will not also be using the SDK error type by that name.

Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

[Copilot]

Pull request overview

This PR fixes an issue where user-defined error enums named "Error" were incorrectly mapped to the built-in soroban_sdk::Error type instead of being treated as user-defined types (UDTs). The fix ensures that all error enums in contract return types are properly represented as UDTs in the contract specification.

Changes:

• Removed special-case handling of "Error" as a built-in type in type mapping
• Added comprehensive test coverage for Result types with user-defined error enums
• Verified the fix works for both "Error" and "MyError" custom error types

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
soroban-sdk-macros/src/map_type.rs	Removed line that mapped "Error" to built-in error type; added unit tests verifying "Error" and "MyError" map to UDTs
tests/add_u64/src/lib.rs	Added two error enums and safe_add functions to test Result type generation
tests/import_contract/src/lib.rs	Added Error enum and functions that use imported contract's error types
tests/add_u64/test_snapshots/test/test_safe_add.1.json	Added ledger snapshot for new test case
soroban-spec-rust/src/lib.rs	Added integration tests verifying correct Result type generation for user-defined errors

[willemneal]

I used this as a test for Claude at finding and fixing the bug I found. I provided the minimal example and it found and fixed it very quickly. And now it being reviewed by another AI which had no comments feels like a new era.

@leighmcculloch My only question is whether there would ever be the case that the Error type in ScTypeDef would be found in a contract spec. Is it supposed to map to soroban_sdk::Error?

[leighmcculloch]

Can you open an issue to go into detail about the problem to make sure we fully capture this bug, it's scope of impact, before we focus on the solution?

We can do some analysis of contracts on mainnet to understand how changing the existing behaviour might break existing contracts, or if there's any impact today that we might need to think differently about.

I suspect this fix is right though when we come back to it.

Side note, this might be resolved by planned work around improving type identities (#1570).

[leighmcculloch]

My only question is whether there would ever be the case that the Error type in ScTypeDef would be found in a contract spec. Is it supposed to map to soroban_sdk::Error?

Yes I think so. Error is an SDK type, it's a valid type to appear at a contract boundary. Whether it is frequently used is a good question though, it may not be.

[leighmcculloch]

Although the Error type captures a much larger range of errors than what a contract fn could return, so there's an argument for not supporting it, or finding some other way to capture that use case. 🤔

[leighmcculloch]

Opened:

• Generated client code uses wrong Error type when user-defined error enum is named "Error" #1710

[leighmcculloch]

See my comment inline. I'd like to flip the solution, same outcome, but more compatible with all existing contracts. Is there a reason you're leaning towards changing how contract specs are built, rather than changing how clients interpret them?

[leighmcculloch]

I think we should do the reverse, emit a non-UDT Error.

Why: to reduce the observable behaviour change in contracts themselves, and shift the change to be in the interpretation of the specs.

So any type named Error always gets stored in the spec as the SDK Error type, which is what happens today.

Then, when a client sees the Error type, they first check for a UDT also named Error and use that instead. This I think shifts all the behaviour change to the CLI, which also means all existing contracts can benefit from this too.

[willemneal]

@leighmcculloch Great point! I updated with an inversion. And updated the generated artifacts since there are now the error types.

[leighmcculloch]

@willemneal could you resolve conflicts? Thanks!

[mootz12]

Closed in favor of #1862