mollie / mollie-api-python

Mollie API client for Python
http://www.mollie.com
BSD 2-Clause "Simplified" License
113 stars 55 forks source link

The RepsonseError object causes erros when depickling #306

Closed kloki closed 1 year ago

kloki commented 1 year ago

The ResponseError can be pickled but will cause an error when depickled.

In our case we encounter this issus when running our payments tasks in Celery. Celery will pickle and depickle the result of a task.

I feel this should be considered a bug because the client will often be run in this distributed worker setup like this.

How to reproduce

import pickle

from mollie.api.error import ResponseError

pickle.loads(pickle.dumps(ResponseError({"detail": "test", "status": "test"})))

will raise

TypeError: string indices must be integers
whyscream commented 1 year ago

Hi @kloki , thanks for your bugreport.

There is no specific code in place for pickling/depickling, since nobody ever needed it. But we can add stuff if needed, of course. I've personally never ran into this issue since we never used Celery (or a different task queue) for processing payments, and fairly: I don't see the use (but you obviously do, as you're stating this would be an often used setup). Can I ask how you are using Celery in payment processing?

kloki commented 1 year ago

IMHO processing payments should always we run in a task queue as much as possible. We're dealing with web calls and they can always fail. The only exception (for us) is create_transaction.

Pickling is used by task queues to store python objects in the external storage like redis or rabbit-mq.

In general pickling should just work and I have never seen the case where is possible to pickle but not depickle.

Our current workaround is to catch the error and raise our own that can be pickled.

whyscream commented 1 year ago

Just to be sure, I'll try to describe the general process:

  1. Customer wants to pay an amount
  2. You create a payment using the Mollie client. I'm guessing this is what you are naming create_transaction. This is a blocking call, which is useful in all cases:
    • An error response can be handled directly, by retrying, or by informing the customer that payment in the requested way is currently not possible.
    • After a successful call, you can send the customer off to the Mollie checkout using the URL in the response.
  3. The customer returns to your application after the payment is completed
  4. You wait for the webhook to report the result of the payment, and report it to the customer, or just ship the goods.

The only place where a task queue could be useful, is while polling for the payment result after step 2 or 3. Maybe you are doing this? Then you should look into https://docs.mollie.com/overview/webhooks. Polling the API is not encouraged by Mollie, webhooks are the way to go.

Additionally some notes on pickling:

As said, we could add a __getstate__ method, but it would still be a risk since you cannot be sure of the payload in the error received from the network.

whyscream commented 1 year ago

This issue is resolved in release 3.3.0, please reopen if you still find any issues.