Skip to content

Conversation

@stevladimir
Copy link
Contributor

@stevladimir stevladimir commented Apr 18, 2025

http-client is kinda a standard for low-level HTTP. req adds some convenience layer over it, but we mostly don't use that. Moreover, its model rather makes things less transparent and more fragile. E.g. we need direct control over HTTP status codes to handle non-success responses, what req obscures (see this thread). servant could also be an option here, but it is too heavy-weight for this use case.
ConnexpayM is also removed. Instead we pass (Config) and return (Response e a) things explicitly. This allows, e.g. to have separate error types for different endpoints. Previously we had single PaymentFailure for all calls, but obviously errors like CVVFailed could not be thrown from, say, void endpoint.
Removal of req also reduces dependency footprint, what is good for a public library.

Other changes done on the way:

  • Dropped "bucks" dependency
    We use neither of its features and all of it is replaced with trivial USD newtype.
  • Log accurately request and response (full body regardless of the size)
    In a public library we should probably use something more flexible than Text -> IO () as a logger, but that is a separate story
  • Build request internals around ToJSON instead of [Pair]
  • Consistent masking of request bodies
  • Factor out dynamic part of the environment (Env) making it part of the request
    Now it is easy to set per-request logger to attach request-specific context to the Connexpay logs

Base automatically changed from stevladimir/tidy-up to master April 22, 2025 09:05
@stevladimir stevladimir force-pushed the stevladimir/refactor branch 6 times, most recently from 5316a3f to b5fc79b Compare April 26, 2025 12:09
@stevladimir stevladimir force-pushed the stevladimir/refactor branch 3 times, most recently from d2c79b8 to 789df13 Compare April 29, 2025 12:32
@stevladimir stevladimir merged commit 9300166 into master Jun 26, 2025
2 checks passed
@stevladimir stevladimir deleted the stevladimir/refactor branch June 26, 2025 16:33
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.

6 participants