Add repr methods for errors

[blueyed]

I think this is very useful when looking at tracebacks with locals.

I will add tests in case you would like to take it.

[brandur-stripe]

Nice.

This seems like an improvement. One question: can you explain the precise reasoning in adding __repr__ in addition to __str__? It seems that in this case __str__ is largely useful for debugging purposes and we should be able to come up with a single display format that's appropriate for both __repr__ and __str__ and save ourselves from maintaining two separate versions.

[blueyed]

It seems that in this case __str__ is largely useful for debugging purposes

Ok, we might go with only __repr__ then?!

[brandur-stripe]

Ok, we might go we only repr then?!

By my reading, if __repr__ is defined and __str__ is not, then Python will fall back to __repr__, so that should be fine.

Is there a good reason to change this though? I would have thought that REPLs and the like would use __str__ if available.

[blueyed]

Yes, we could also improve __str__ (AFAIK), but from my point of view __repr__ should reflect the important parts from the constructor, while __str__ is more user-facing.
Therefore I've thought initially to keep __str__ like it is, especially since this might somehow displayed to users after all.
__repr__ however is then useful in the interactive shell, or for locals in a traceback.

[brandur-stripe]

Yes, we could also improve str (AFAIK), but from my point of view repr should reflect the important parts from the constructor, while str is more user-facing.

My intuition is that there isn't enough of a distinction here to merit the two separate implementations — __str__ is already showing a request ID and a Stripe error message, which is probably more information than you'd want to expose.

Let's pick either one or the other, add a little bit of testing, and we can get this in.

[blueyed]

Cool. I'll try __repr__ alone then, and maybe that helps already with ditching the py2/py3 specific code (hopefully).

[brandur-stripe]

Cool. I'll try repr alone then, and maybe that helps already with ditching the py2/py3 specific code (hopefully).

Sounds good.

It may also be worth taking a peek at #364 which could come in soon, and which will aim to make stripe-python agnostic to Python 2 versus 3 by using six. If making significant changes, you mighttt to consider basing off of that branch.

[blueyed]

Do we want to keep the existing str format?

stripe-python/tests/test_error.py

Lines 19 to 33 in c3513fb

	def test_formatting_with_request_id(self):
	err = StripeError(u'öre', headers={'request-id': '123'})
	if six.PY3:
	assert str(err) == u'Request 123: öre'
	else:
	assert str(err) == 'Request 123: \xc3\xb6re'
	assert unicode(err) == u'Request 123: öre'
	def test_formatting_with_none(self):
	err = StripeError(None, headers={'request-id': '123'})
	if six.PY3:
	assert str(err) == u'Request 123: <empty message>'
	else:
	assert str(err) == 'Request 123: <empty message>'
	assert unicode(err) == u'Request 123: <empty message>'

btw: those tests look like they are expecting pytest (bare assert), but tests are only run with unittest, right? (and therefore assertEquals should be used?!)

[blueyed]

Somebody might inspect them for "empty message" and/or 'Request \d+:' etc.

[blueyed]

For B/C I think __str__ should be kept after all.

[blueyed]

Is this good for py2?

[blueyed]

Rebased.
Only fails for py26, which is meant to be dropped (#386).

Came back to this since a better repr helps a lot with locals in tracebacks, so please review/answer my previous comments/questions.

[ob-stripe]

Hey @blueyed, thanks a lot for the contribution.

So I think there are two options here:

• updating the implementation to format the repr strings using the % syntax rather than .format()
• rebasing the PR against the integration-v2 branch rather than master

If you're in a hurry to see this merged in, the first option is probably better since I'm not sure when 2.0 will be released (there are still a bunch of updates I want to include, but this is fairly low on my priority list right now).

[blueyed]

Rebased this on master, using % formatting.

[ob-stripe]

Thanks @blueyed! Released as 1.82.2.