classification
Title: os.environ converts key type from string to bytes in KeyError exception
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.4, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Drekin, Robert.Tasarz, haypo, python-dev, r.david.murray
Priority: normal Keywords: patch

Created on 2013-04-12 00:36 by Robert.Tasarz, last changed 2013-08-23 17:27 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
os_environ_keyerror.patch haypo, 2013-04-12 08:43 review
os_environb_raise_from_none.patch haypo, 2013-08-23 15:12 review
Messages (12)
msg186604 - (view) Author: Robert Tasarz (Robert.Tasarz) Date: 2013-04-12 00:36
Minimal example:

>>> import os
>>> somekey = 'random'
>>> try:
...   os.environ[somekey]
... except KeyError as e:
...   print(repr(e))
...   somekey == e.args[0]
... 
KeyError(b'random',)
False

Tested in Python 3.3.1 on Debian
msg186612 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-04-12 06:23
Well, it is not that it is converting it in the exception, it is that it is converting it in order to do the lookup.  It is an interesting question whether or not its value in the exception should be considered a bug.
msg186613 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-04-12 08:43
"It is an interesting question whether or not its value in the exception should be considered a bug."

I consider it as a bug because it is very surprising to have a different key on the error. Attached patch fixes the issue.
msg186620 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-04-12 13:03
Sounds fine to me.

Is there a reason you are not using 'with assertRaises' in the test?
msg186626 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-04-12 13:36
> Is there a reason you are not using 'with assertRaises' in the test?

The test is not testing the exception class, but the first argument of
the exception. How do you test that using assertRaises()? Especially
to check "err.args[0] is missing".
msg186634 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-04-12 14:09
with self.assertRaises(KeyError) as cm:
    os.environ[missing]
self.assertEqual(cm.excecption.args[0], missing)

(I don't know why assertRaises returns itself rather than just returning the exception in the with, but that's the API).
msg186923 - (view) Author: Roundup Robot (python-dev) Date: 2013-04-14 14:44
New changeset 72df981e83d3 by Victor Stinner in branch '3.3':
Close #17702: os.environ now raises KeyError with the original environment
http://hg.python.org/cpython/rev/72df981e83d3

New changeset ea54559a4442 by Victor Stinner in branch 'default':
(Merge 3.3) Close #17702: os.environ now raises KeyError with the original
http://hg.python.org/cpython/rev/ea54559a4442

New changeset b0c002fa4335 by Victor Stinner in branch '3.3':
Issue #17702: use assertRaises() for the unit test
http://hg.python.org/cpython/rev/b0c002fa4335

New changeset cc6c5b5ec4f2 by Victor Stinner in branch 'default':
(Merge 3.3) Issue #17702: use assertRaises() for the unit test
http://hg.python.org/cpython/rev/cc6c5b5ec4f2
msg195973 - (view) Author: Adam Bartoš (Drekin) * Date: 2013-08-23 14:20
This patch introduces a bit ugly traceback. Shouldn't there be “raise from None” or “raise from previous_exc” instead of simple raise?
msg195980 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-08-23 15:12
"This patch introduces a bit ugly traceback. Shouldn't there be “raise from None” or “raise from previous_exc” instead of simple raise?"

Oh, I see.

>>> os.environb[b'10']
Traceback (most recent call last):
  File "/home/vstinner/prog/python/default/Lib/os.py", line 648, in __getitem__
    value = self._data[self.encodekey(key)]
KeyError: b'10'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/vstinner/prog/python/default/Lib/os.py", line 651, in __getitem__
    raise KeyError(key)
KeyError: b'10'

Attached patch adds "from None".
msg195981 - (view) Author: Adam Bartoš (Drekin) * Date: 2013-08-23 15:32
It's also in __delitem__.
msg195987 - (view) Author: Roundup Robot (python-dev) Date: 2013-08-23 17:24
New changeset 26c049dc1a4a by Victor Stinner in branch '3.3':
Close #17702: On error, os.environb now removes suppress the except context
http://hg.python.org/cpython/rev/26c049dc1a4a

New changeset 01f33959ddf6 by Victor Stinner in branch 'default':
(Merge 3.3) Close #17702: On error, os.environb now removes suppress the except
http://hg.python.org/cpython/rev/01f33959ddf6
msg195990 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-08-23 17:27
> This patch introduces a bit ugly traceback.

It is now fixed, thanks for the report!
History
Date User Action Args
2013-08-23 17:27:48hayposetmessages: + msg195990
2013-08-23 17:24:48python-devsetstatus: open -> closed
resolution: fixed
messages: + msg195987
2013-08-23 15:32:10Drekinsetmessages: + msg195981
2013-08-23 15:12:35hayposetstatus: closed -> open
files: + os_environb_raise_from_none.patch
resolution: fixed -> (no value)
messages: + msg195980
2013-08-23 14:20:30Drekinsetnosy: + Drekin
messages: + msg195973
2013-04-14 14:44:00python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg186923

resolution: fixed
stage: resolved
2013-04-12 14:09:38r.david.murraysetmessages: + msg186634
2013-04-12 13:36:23hayposetmessages: + msg186626
2013-04-12 13:03:42r.david.murraysetmessages: + msg186620
2013-04-12 08:43:35hayposetfiles: + os_environ_keyerror.patch
keywords: + patch
messages: + msg186613

versions: + Python 3.4
2013-04-12 06:23:07r.david.murraysetnosy: + r.david.murray
messages: + msg186612
2013-04-12 01:30:05ned.deilysetnosy: + haypo
2013-04-12 00:37:32Robert.Tasarzsettype: behavior
2013-04-12 00:36:11Robert.Tasarzcreate