bug in error path of cross_check_pkg_resources_versus_import #1355

New Issue

warner · 2011-02-03T21:52:59Z

warner commented

2011-02-03 21:52:59 +00:00

Jimmy Tang wrote to the mailing list with a crash on his Archlinux box.

  File "/home/jtang/tmp/tahoe-lafs/src/allmydata-tahoe-1.8.2/src/allmydata/__init__.py",
line 301, in cross_check_pkg_resources_versus_import
    % (imp_ver, name, imp_loc, pr_ver, pr_loc, e.__class__.name, e))
AttributeError: type object 'exceptions.TypeError' has no attribute 'name'
    Aborting...

I don't know if this is a real error path, or if it's just accumulating a bunch of warnings to print to stderr (i.e. if we just took out the offending line, would the program work anyways, or is the missing/old/unable-to-compute-version dependency actually important?).

This sort of problem reinforces my feeling that our *init*.py is way too complex, and that all the version-checking logic may be doing more harm than good.

This may provoke a 1.8.3, if it happens frequently enough. If the bad e.*class*.name is only triggered under weird combinations of installed dependencies, then we might be able to put it off until 1.9.0 .

Jimmy Tang [wrote](http://tahoe-lafs.org/pipermail/tahoe-dev/2011-February/006074.html) to the mailing list with a crash on his Archlinux box. ``` File "/home/jtang/tmp/tahoe-lafs/src/allmydata-tahoe-1.8.2/src/allmydata/__init__.py", line 301, in cross_check_pkg_resources_versus_import % (imp_ver, name, imp_loc, pr_ver, pr_loc, e.__class__.name, e)) AttributeError: type object 'exceptions.TypeError' has no attribute 'name' Aborting... ``` I don't know if this is a real error path, or if it's just accumulating a bunch of warnings to print to stderr (i.e. if we just took out the offending line, would the program work anyways, or is the missing/old/unable-to-compute-version dependency actually important?). This sort of problem reinforces my feeling that our `*init*.py` is way too complex, and that all the version-checking logic may be doing more harm than good. This may provoke a 1.8.3, if it happens frequently enough. If the bad `e.*class*.name` is only triggered under weird combinations of installed dependencies, then we might be able to put it off until 1.9.0 .

warner added the

labels 2011-02-03 21:52:59 +00:00

warner added this to the 1.9.0 milestone 2011-02-03 21:52:59 +00:00

davidsarah commented

2011-02-04 02:17:49 +00:00

warner wrote in the original Description:

I suspect it's a python-2.7 incompatibility in the error-handling path in all the crazy what-version-of-each-dependency code inside *init*.py:

...

I suspect that TypeError moved from being implemented in Python (in py2.6) to being implemented in C (in py2.7), and that for some reason when you try to do the .name lookup, it explodes messily.

It should have been e.*class*.*name*. Sorry, my fault for not testing this error path adequately. It's not at all Python 2.7-specific.

(For future reference: the Description field isn't the right place to speculate about the cause of a bug.)

This may provoke a 1.8.3, if it happens frequently enough. If the bad e.*class*.name is only triggered under weird combinations of installed dependencies, then we might be able to put it off until 1.9.0 .

This error occurs on lines 287 and 301 of [init.py]source:src/allmydata/init.py@4994#L279. It happens when:

the --version or --version-and-path option to bin/tahoe is used, and
the version of a package obtained either from import or from pkg_resources, is not parseable at all by source:src/allmydata/util/verlib.py. (A non-normalized but parseable version is fine.)

--version-and-path is passed to bin/tahoe by default when you run tests [source:setup.py@4997#L258 via setup.py]. You can use the --quiet option (i.e. setup.py test --quiet or setup.py trial --quiet) to suppress this.

I don't know if this is a real error path, or if it's just accumulating a bunch of warnings to print to stderr (i.e. if we just took out the offending line, would the program work anyways, or is the missing/old/unable-to-compute-version dependency actually important?).

We don't know whether the warning was indicative of a real error in this case, because we don't know what the version or package was. The best way to check that would be for Jimmy Tang to replace .name with .__name__ on lines 287 and 301 of __init__.py and then see what is printed.

It's not clear to me how common it is to have versions that verlib.py will raise an exception when trying to parse.

warner wrote in the original Description: > I suspect it's a python-2.7 incompatibility in the error-handling path in all the crazy what-version-of-each-dependency code inside `*init*.py`: > > ... > > I suspect that `TypeError` moved from being implemented in Python (in py2.6) to being implemented in C (in py2.7), and that for some reason when you try to do the `.name` lookup, it explodes messily. It should have been `e.*class*.*name*`. Sorry, my fault for not testing this error path adequately. It's not at all Python 2.7-specific. (For future reference: the Description field isn't the right place to speculate about the *cause* of a bug.) > This may provoke a 1.8.3, if it happens frequently enough. If the bad `e.*class*.name` is only triggered under weird combinations of installed dependencies, then we might be able to put it off until 1.9.0 . This error occurs on lines 287 and 301 of [*init*.py]source:src/allmydata/*init*.py@4994#L279. It happens when: * the `--version` or `--version-and-path` option to `bin/tahoe` is used, and * the version of a package obtained either from import or from pkg_resources, is not parseable at all by source:src/allmydata/util/verlib.py. (A non-normalized but parseable version is fine.) `--version-and-path` is passed to `bin/tahoe` by default when you run tests [source:setup.py@4997#L258 via setup.py]. You can use the `--quiet` option (i.e. `setup.py test --quiet` or `setup.py trial --quiet`) to suppress this. > I don't know if this is a real error path, or if it's just accumulating a bunch of warnings to print to stderr (i.e. if we just took out the offending line, would the program work anyways, or is the missing/old/unable-to-compute-version dependency actually important?). We don't know whether the warning was indicative of a real error in this case, because we don't know what the version or package was. The best way to check that would be for Jimmy Tang to replace `.name` with `.__name__` on lines 287 and 301 of `__init__.py` and then see what is printed. It's not clear to me how common it is to have versions that `verlib.py` will raise an exception when trying to parse.

tahoe-lafs changed title from ~~py2.7 incompatibility in cross_check_pkg_resources_versus_import?~~ to bug in error path of cross_check_pkg_resources_versus_import

2011-02-04 02:17:49 +00:00

davidsarah commented

2011-02-04 02:36:18 +00:00

Replying to davidsarah:

This error [...] happens when:

the --version or --version-and-path option to bin/tahoe is used, and

the version of a package obtained either from import or from pkg_resources, is not parseable at all by source:src/allmydata/util/verlib.py. (A non-normalized but parseable version is fine.)

Oops, cross_check_pkg_resources_versus_import is also called by [get_package_versions_string]source:src/allmydata/init.py@4994#L378, which is used [when constructing a storage client or introducer node]source:src/allmydata/node.py@4852#L4775. This will prevent running nodes whenever the second condition above holds.

This situation is not happening on any of the buildbots, so it can't be very common. However I think it still requires a 1.8.3 :-(

Replying to [davidsarah](/tahoe-lafs/trac-2024-07-25/issues/1355#issuecomment-82607): > This error [...] happens when: > * the `--version` or `--version-and-path` option to `bin/tahoe` is used, and > * the version of a package obtained either from import or from pkg_resources, is not parseable at all by source:src/allmydata/util/verlib.py. (A non-normalized but parseable version is fine.) Oops, `cross_check_pkg_resources_versus_import` is also called by [get_package_versions_string]source:src/allmydata/*init*.py@4994#L378, which is used [when constructing a storage client or introducer node]source:src/allmydata/node.py@4852#L4775. This will prevent running nodes whenever the second condition above holds. This situation is not happening on any of the buildbots, so it can't be very common. However I think it still requires a 1.8.3 :-(

zooko commented

2011-02-20 23:33:14 +00:00

I think this is so rare that it doesn't require 1.8.3.

It is quite rare for one of our deps to have a version number that can't be parsed by verlib.py.

I think this is so rare that it doesn't require 1.8.3. It is quite rare for one of our deps to have a version number that can't be parsed by verlib.py.

davidsarah commented

2011-02-21 02:03:34 +00:00

Attachment test-1355.darcs.patch (6439 bytes) added

Add unit tests for cross_check_pkg_resources_versus_import, and a regression test for ref #1355. This requires a little refactoring to make it testable.

**Attachment** test-1355.darcs.patch (6439 bytes) added Add unit tests for cross_check_pkg_resources_versus_import, and a regression test for ref #1355. This requires a little refactoring to make it testable.

test-1355.darcs.patch

6.3 KiB

davidsarah commented

2011-02-21 02:04:30 +00:00

Attachment fix-1355.darcs.patch (6305 bytes) added

allmydata/init.py: .name was used in place of the correct .name when printing an exception. Also, robustify string formatting by using %r instead of %s in some places. fixes #1355.

**Attachment** fix-1355.darcs.patch (6305 bytes) added allmydata/*init*.py: .name was used in place of the correct .*name* when printing an exception. Also, robustify string formatting by using %r instead of %s in some places. fixes #1355.

fix-1355.darcs.patch

6.2 KiB

david-sarah@jacaranda.org commented

2011-02-21 02:16:08 +00:00

In changeset:71c301ca3444f41e:

allmydata/__init__.py: .name was used in place of the correct .__name__ when printing an exception. Also, robustify string formatting by using %r instead of %s in some places. fixes #1355.

In changeset:71c301ca3444f41e: ``` allmydata/__init__.py: .name was used in place of the correct .__name__ when printing an exception. Also, robustify string formatting by using %r instead of %s in some places. fixes #1355. ```

tahoe-lafs added the

fixed

label 2011-02-21 02:16:08 +00:00

david-sarah@jacaranda.org closed this issue

2011-02-21 02:16:08 +00:00

davidsarah commented

2011-02-21 05:40:50 +00:00

The test patch was applied as changeset:787d12165af49a4a (trac didn't auto-comment for some reason).

Sign in to join this conversation.