Skip to content

Reliability fixes in SaferPayClient#8

Merged
viwodev merged 1 commit into
viwodev:mainfrom
clementfavre:fix/saferpayclient-reliability
May 18, 2026
Merged

Reliability fixes in SaferPayClient#8
viwodev merged 1 commit into
viwodev:mainfrom
clementfavre:fix/saferpayclient-reliability

Conversation

@clementfavre
Copy link
Copy Markdown
Collaborator

Hi, bundled four small things I noticed in SaferPayClient.cs.

  • The Dispose method calls itself in its own body, so any consumer doing using var client = new SaferPayClient(...) crashes with a StackOverflowException at end of scope. The class doesn't hold anything to release, so I just emptied it
  • The 4-arg constructor never assigns _jsonSerializerSettings, while the other two do. It happens to be the one Settings.Client() and every Channel shortcut use, so most consumers end up with a null value and Newtonsoft silently falls back to its own defaults. I chained it to the existing 2-arg ctor
  • All the Send, Get, Post and Delete methods create a RestClient without ever disposing it. Since RestSharp wraps an HttpClient, that leaks sockets under load. I added using on each one
    And in the catch blocks of SendAsync, two throw spex / throw ex statements were resetting the stack trace, so I changed them to plain throw

@viwodev viwodev merged commit 2780300 into viwodev:main May 18, 2026
2 checks passed
@viwodev
Copy link
Copy Markdown
Owner

viwodev commented May 18, 2026

Thanks a lot for the detailed PR and explanations.

All of the reported issues made sense and were reproducible on my side as well. The fixes around Dispose(), serializer initialization, RestClient disposal and stack trace preservation have now been merged into main, along with a few additional cleanup and reliability improvements afterward.

Really appreciate you taking the time to investigate and contribute 🚀

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