Skip to content

refactor!: introduces namespacing, error classes and renames legacy types#14

Merged
lucasloisp merged 25 commits intomainfrom
feature/sdk-cleanupv1
Mar 12, 2026
Merged

refactor!: introduces namespacing, error classes and renames legacy types#14
lucasloisp merged 25 commits intomainfrom
feature/sdk-cleanupv1

Conversation

@andreydobrikov
Copy link
Collaborator

This pull request introduces several improvements and refactors to the Privy SDK, focusing on code style enforcement, namespace organization, and authentication logic. The main changes include strengthening code style guidelines, updating documentation, consolidating analytics and authentication namespaces, and refactoring exception handling and session management in the authentication flow.

Code style and documentation updates:

  • Enhanced code style enforcement in .editorconfig to prefer auto-properties and expression-bodied methods, improving readability and maintainability.
  • Updated CONTRIBUTING.md to clarify formatting rules and highlight key style preferences, making contribution guidelines clearer for new developers.

Namespace and interface organization:

  • Refactored analytics-related files (ClientAnalyticsIdRepository, IAnalyticsManager, IAnalyticsRepository, IClientAnalyticsIdRepository) to use the Privy.Analytics namespace for better modularity and clarity. [1] [2] [3] [4]
  • Updated authentication files (AuthDelegator, AuthRepository) to use Privy.Auth and related namespaces, aligning with new project structure. [1] [2]

Authentication logic and error handling:

  • Replaced PrivyException.AuthenticationException with PrivyAuthenticationException throughout AuthDelegator, standardizing exception handling for authentication errors. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13]
  • Refactored session update logic in AuthDelegator to use a strongly-typed SessionUpdateAction enum instead of string literals, reducing bugs and improving code clarity.

SDK usage and documentation improvements:

  • Improved SDK/README.md with clearer initialization instructions, explanation of authentication state handling, and updated wallet interface references for accuracy. [1] [2] [3]

Model and interface improvements:

  • Changed AnalyticsEventRequestData properties to use C# auto-properties for consistency with code style rules.

These changes collectively improve code clarity, maintainability, and developer experience for the Privy SDK.

Added null‑checks/try‑catches in token helpers and unsubscribed auth event
Corrected .editorconfig key
Hardened README samples
Added InternalError enum & refined OTP error mapping
Used SafeFireAndForget with logging for initialization
Fixed misleading exception message in wallet manager
EmbeddedWallets  is deprecated, use EmbeddedEthereumWallets now.
Updated PrivyUser to use new EmbeddedEthereumWallets
Harden exception handling by using numeric status codes instead of parsing string values.
SDK/README.md Outdated
Comment on lines +56 to +59
case AuthState.Authenticated:
// User is authenticated. Grab the user's linked accounts
var privyUser = await PrivyManager.Instance.GetUser();
var linkedAccounts = privyUser.LinkedAccounts;
var user = await PrivyManager.Instance.GetUser();
var linkedAccounts = user?.LinkedAccounts; // user is null if unauthenticated
Copy link
Collaborator

@lucasloisp lucasloisp Mar 12, 2026

Choose a reason for hiding this comment

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

This one's tough but given we know privyUser to be non-null here (because AuthState is authenticated: what do you think of removing the ?. check? I'd be fine with ! here.

An alternative from our other client SDKs is giving AuthState itself a User property that's always non-null in the Authenticated case.

Something potentially like this:

public abstract record AuthState
{
    // Prevent external inheritance to mimic the closed nature of an enum
    private AuthState() { }

    public record NotReady : AuthState;

    public record Unauthenticated : AuthState;

    public record Authenticated(IPrivyUser User) : AuthState;
}

Feel free to disagree! If we do like this: let's do in a separate PR 🙏

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree, this maybe too defensive here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, I think it's in the doc and we don't actually guard in the code anymore. Example here from our actual code.

            if (state == AuthState.Authenticated)
            {
                var user = await PrivyManager.Instance.GetUser();
                Debug.Log($"[privy.status] User ID: {user?.Id ?? "N/A"}");
            }

{
internal static class LinkedAccountResponseMapper
{
private static LinkedAccountType ParseType(string type)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this static method live instead in the LinkedAccountType class itself?

`LinkedAccountType.FromString(string type) -> LinkedAccountType

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can see a reasoning for that, but prefer to keep it here since it does not spill into public API and is clear responsibility at the response parsing level.

Comment on lines +313 to +314
throw new PrivyWalletException($"Failed to sign with user signer",
EmbeddedWalletError.CreateAdditionalFailed); //Let this bubble up to HandleAuthStateChanged and AwaitConnected
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's adjust the wording to "authorization key" over "signer"! That's the term we're using now in docs.

also the enum error code.

Suggested change
throw new PrivyWalletException($"Failed to sign with user signer",
EmbeddedWalletError.CreateAdditionalFailed); //Let this bubble up to HandleAuthStateChanged and AwaitConnected
throw new PrivyWalletException($"Failed to sign with user's authorization key",
EmbeddedWalletError.UserSignerRequestFailed); //Let this bubble up to HandleAuthStateChanged and AwaitConnected

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Going to make this change as part of a separate PR since I refactored this class a bit and this is not longer relevant.

andreydobrikov and others added 3 commits March 12, 2026 11:44
Remove extra comment

Co-authored-by: Lucas Lois <lucas.lois@privy.io>
Explicit types

Co-authored-by: Lucas Lois <lucas.lois@privy.io>
removed internal comment

Co-authored-by: Lucas Lois <lucas.lois@privy.io>
@lucasloisp lucasloisp changed the title SDK Refactor v1.0 feat!: introduces namespacing, error classes and Mar 12, 2026
@lucasloisp lucasloisp changed the title feat!: introduces namespacing, error classes and refactor!: introduces namespacing, error classes and renames legacy types Mar 12, 2026
@lucasloisp lucasloisp merged commit 99bc653 into main Mar 12, 2026
4 checks passed
@lucasloisp lucasloisp deleted the feature/sdk-cleanupv1 branch March 12, 2026 19:15
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.

2 participants