Deleting object.metadata[key] doesn't work anymore

[adrienverge]

Python 3.10
stripe-python versions tested: 2.1.0, 2.20.0, 2.33.0, 2.63.0 (latest)
Stripe API 2020-08-27 (latest)

Hello,

Deleting customer metadata using = None or del metadata[key] does not work anymore. It used to work, as this other issue suggests.

Strangely, all stripe-python versions tested (from 2.1.0 to 2.63.0) have the bug. This could show a change on the Stripe API, but this doesn't seem to be the case because using curl instead of the Python lib works (see examples below).

How to reproduce:

customer = stripe.Customer.create(metadata={'a': 1, 'b': 2})
print(customer.metadata)
# {
#   "a": "1",
#   "b": "2"
# }
# → THIS IS NORMAL

customer.metadata['b'] = None
customer = stripe.Customer.modify(customer.id, metadata=customer.metadata)
print(customer.metadata)
# {
#   "a": "1",
#   "b": "2"
# }
# → EXPECTED VALUE: {"a": 1}

del customer.metadata['b']
customer = stripe.Customer.modify(customer.id, metadata=customer.metadata)
print(customer.metadata)
# {
#   "a": "1",
#   "b": "2"
# }
# → EXPECTED VALUE: {"a": 1}

customer = stripe.Customer.retrieve(customer.id)
print(customer.metadata)
# {
#   "a": "1",
#   "b": "2"
# }
# → EXPECTED VALUE: {"a": 1}

customer.metadata['b'] = ''
customer = stripe.Customer.modify(customer.id, metadata=customer.metadata)
# → FAILS WITH: ValueError: You cannot set b to an empty string. We interpret
# empty strings as None in requests.You may set {"a": "1", "b": "2"}.b = None to
# delete the property

However, using a raw HTTP request successfully removes b from metadata:

curl -u rk_test_…: 'https://api.stripe.com/v1/customers/cus_…' -d 'metadata[b]'=

[dcr-stripe]

Thanks for the report @adrienverge.

I think I know what's going on here... deleting a metadata key is handled differently if you're using a flow that relies on mutating an object and calling save or a flow that uses the modify endpoints.

Looking at the options outlined in #596:

---

Option 1: Mutate + save()

pi.metadata['damage_deposit_amount'] = None
pi.save()

Notice that this uses a save() call instead of modify. The save() route still works today and will delete that metadata key.

However you're right that del does not work correctly with the metadata object when using save():

customer.metadata['b'] = None
customer.save()

customer = stripe.Customer.retrieve(customer.id)
print(customer.metadata)
# {
#  "a": "1",
# }

vs.

del customer.metadata['b']
customer.save()

customer = stripe.Customer.retrieve(customer.id)
print(customer.metadata)
# {
#  "a": "1",
#  "b": "2",
# }

I don't think del has ever worked. This is because del removes it entirely from the map while = None does not. In that linked issue, we only fixed it so that del would not raise an error, as per @ob-stripe 's message, but this was not intended to be the mechanism for deleting items from metadata (= None is, in this flow). The reasoning here is that if the key is not present at all in the map, we have no way of knowing the user wants to delete it or wants to do a partial update (hence why you have to explicitly use = None).

---

Option 2: modify

stripe.PaymentIntent.modify('pi_deadbeef', metadata={'damage_deposit_amount': ''})

This still works, and is different than your code which actually mutates the local Stripe object and encounters that mentioned ValueError:

payment_intent.metadata['damage_deposit_amount'] = ''
stripe.PaymentIntent.modify(...)

This error message is very confusing... = None to delete the property is meant to be used with save(). For modify, we use the empty string.

As an example, this works:

customer = stripe.Customer.create(metadata={'a': 1, 'b': 2})
print(customer.metadata)

customer = stripe.Customer.modify(customer.id, metadata={'b': ''})

customer = stripe.Customer.retrieve(customer.id)
print(customer.metadata)
# {
#   "a": "1",
# }

If you try using metadata={'b': None} for the modify call, it gets ignored. The Ruby SDK works similarly:

c = Stripe::Customer.create({
  metadata: {'a': '1', 'b': '2'},
})

Stripe::Customer.update(c.id, {metadata: {'b': nil}})

c2 = Stripe::Customer.retrieve(c.id)

puts c2.metadata
# {
#  "a": "1",
#  "b": "2"
# }

vs.

c = Stripe::Customer.create({
  metadata: {'a': '1', 'b': '2'},
})

Stripe::Customer.update(c.id, {metadata: {'b': ''}})

c2 = Stripe::Customer.retrieve(c.id)
puts c2.metadata
# {
#  "a": "1"
# }

---

To summarize, there's a few concepts being mixed in here:

• To delete a key using save(), we expect the metadata object to be mutated and the value set to None.
  • del has never worked and we can't support it since we need to be able to differentiate between the user actually wanting to delete the metadata key and the user wanting to do a partial metadata update.
  • If you mutate the object and use empty string instead of None, an error appears.
• To delete a key using modify(), we expect a new metadata object to have an empty string for the key.
  • Using None as the value gets ignored.

The error message is very confusing here and we could likely clarify.

[dcr-stripe]

I've clarified the deletion message. Going to close this as there is no regression (del did not truly work to begin with) and we are currently consistent with the other SDKs by using the empty string.

[adrienverge]

Thanks for this detailed answer, which is really helpful to understand the problem 👍 👍

OK for del.

For = None + modify(), based on your answer I think I can write a workaround by cloning the metadata object before deleting keys.

However, in my opinion the way stripe-python handles metadata is not consistent and confusion-prone (i.e. I still see a "bug"):

• Either passing a None value in modify() should delete the field.

• Or passing a None value in modify() does nothing at all (like the Ruby lib), but in this case it should be allowed to set original_metadata[key] = "" to delete the field. Currently it crashes.

  It shouldn't be required from developers to know that metadata should be cloned into new dict priorly.

  Could this be easily handled on stripe-python side, by cloning metadata inside the lib?

[dcr-stripe]

Or passing a None value in modify() does nothing at all (like the Ruby lib), but in this case it should be allowed to set original_metadata = "" to delete the field. Currently it crashes.

This is only happening because you're directly mutating the dict on customer. If you clone customer.metadata and then set a value there to be ="", modify() will work as expected. The crash happens at the mutation itself, not in the modify() call.

In general, treating the received Stripe objects as immutable is the way to go if you plan on using modify(). This is a known thorny part of our SDKs, but unfortunately making them all immutable in practice would require a breaking change. I agree it's not ideal in the interim.

[adrienverge]

OK, makes sense.

It's still confusing to be allowed to mutate the dict to = None (without an effect when calling modify()) but not being able to mutate it to = "" 😉

[databasedav]

arrived here from attempting to do a .modify(metadata=None) and nothing happening, it's caused by the behavior of stripe.api_requestor._api_encode which removes keywords with a None value entirely, i think if None is provided, it should be changed to '' automatically to make the api more ergonomic