Skip to content

Fixes wrong error messages shown#33

Open
acogn wants to merge 14 commits into
stripe:mainfrom
acogn:main
Open

Fixes wrong error messages shown#33
acogn wants to merge 14 commits into
stripe:mainfrom
acogn:main

Conversation

@acogn

@acogn acogn commented Mar 7, 2026

Copy link
Copy Markdown

Summary

  • Fixes Error messages incorrectly reports missing wallet and hides actual incompatibility reason #32
  • preserve incompatibility reason in payment negotiation instead of collapsing all failures into a wallet-missing style error
  • add reason metadata to PurlError::NoCompatibleMethod (MissingWallet, UnsupportedToken, NetworkFiltered)
  • keep typed PurlError in CLI (anyhow::Error::from) so reason-aware suggestions can be shown
  • update NoCompatibleMethod suggestions/related commands to reflect the real cause
  • add regression tests for negotiator reason selection and CLI suggestion rendering

Bug

When a server returns a payment requirement that is incompatible for non-wallet reasons (for example unsupported token), purl reported a wallet-missing style error even when a wallet was configured.

This happened because negotiation reduced compatibility checks to a boolean path and discarded the underlying cause, then CLI stringified the error and lost structured context.

Verification

  • cargo test -p purl-lib negotiator::tests
  • cargo test -p purl errors::tests
  • cargo run -- --dry-run https://www.getblauwdruk.com/for-agents/api
  • now reports unsupported token configuration (USDT on Solana) instead of incorrectly saying no wallet is configured

…thod errors

Improve payment requirement negotiation and CLI error reporting so
that error messages reflect the actual cause instead of
defaulting to wallet-missing messaging.

- Add CompatibilityReason metadata to PurlError::NoCompatibleMethod
  (missing wallet, unsupported token, network filtered)
- Preserve reason data in PaymentNegotiator selection flow
- Keep typed PurlError in CLI (avoid stringifying into anyhow) so
  suggestions can downcast and render reason-specific guidance
- Update NoCompatibleMethod suggestions/related commands in CLI
- Add negotiator and CLI tests for reason-aware behavior

This fixes cases where a configured Solana wallet was reported as missing
when the real issue was unsupported token configuration.
@cla-assistant

cla-assistant Bot commented Mar 7, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@acogn acogn changed the title Fixes stripe/purl#32 Fixes wrong error messages shown Mar 8, 2026
pejmanjohn and others added 7 commits March 9, 2026 16:45
@stevekaliski-stripe

Copy link
Copy Markdown
Contributor

@acogn overall looks good! there are some merge conflicts!

stevekaliski-stripe and others added 3 commits March 11, 2026 15:25
…thod errors

Improve payment requirement negotiation and CLI error reporting so
that error messages reflect the actual cause instead of
defaulting to wallet-missing messaging.

- Add CompatibilityReason metadata to PurlError::NoCompatibleMethod
  (missing wallet, unsupported token, network filtered)
- Preserve reason data in PaymentNegotiator selection flow
- Keep typed PurlError in CLI (avoid stringifying into anyhow) so
  suggestions can downcast and render reason-specific guidance
- Update NoCompatibleMethod suggestions/related commands in CLI
- Add negotiator and CLI tests for reason-aware behavior

This fixes cases where a configured Solana wallet was reported as missing
when the real issue was unsupported token configuration.
@acogn

acogn commented Mar 11, 2026

Copy link
Copy Markdown
Author

@stevekaliski-stripe should be fixed now

Btw nice demo video!

@stevekaliski-stripe

Copy link
Copy Markdown
Contributor

@acogn some test failures!

@acogn

acogn commented Mar 12, 2026

Copy link
Copy Markdown
Author

@stevekaliski-stripe I fixed the flaky test failure by addressing a race condition on HOME. I changed it to a a shared serial_test key (home_env) to make them deterministic.

Note that this issue was unrelated to the earlier changes in the PR.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error messages incorrectly reports missing wallet and hides actual incompatibility reason

5 participants