Page MenuHomePhabricator

Report sensible error when family file cannot be loaded
Closed, DeclinedPublic

Description

"pywikibot.exceptions.Error: Family dibaf does not exist" is not a helpful message if the problem really is

'cannot import family' (because someone tries 'import family' instead of 'from pywikibot import family'

or something like that.


Version: core-(2.0)
Severity: enhancement

Details

Reference
bz58878

Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 22 2014, 2:20 AM
bzimport set Reference to bz58878.
bzimport added a subscriber: Unknown Object (????).

Family.load uses

        mod = imp.load_source(fam, config.family_files[fam])
except (ImportError, KeyError):
    raise UnknownFamily(u'Family %s does not exist' % fam)

So I believe this bug is asking for the KeyError to be separated from the ImportError.

Alternatively, separating the ImportError caused by 'I can't find the file families/wikidata_family.py' from an ImportError caused by 'I tried to load families/wikidata_family.py, but there's a syntax error, or I can't load a dependency, or ...'

I think separating the ImportError and KeyError is probably good enough, or even just adding the exception message to the re-raised exception.

Ideally we create a new Exception class for the ImportError, which is subclass of UnknownFamily and ImportError, and it includes the original ImportError message

I think it's catching the ImportError in case someone added their own family file to config.family_files. I actually think that we shouldn't even catching the KeyError there but doing that separately so that any KeyError thrown while importing it isn't caught. Unfortunately I don't think it's possible to separate between errors during import (where the family file is problematic) and errors “initializing” the import (e.g. when load_source raised the ImportError directly).

T104130 will solve this automatically, but IMO it would be good to fix this issue separately first, so we can bikeshed details like the new error messages/exception classes here.

Xqt subscribed.

This is very outdated and the exceptions are handled in a different way already