[PATCH] Workflow: "KeyError: oldstates" could be reported more gracefully

[Steven R. Loomis <srl@…>]

This is total user error, but if a workflow item doesn't have any states:

[ticket-workflow]
#accept = new,reviewing -> assigned    <<<<< OOPS!
accept.operations = set_owner_to_self
accept.permissions = TICKET_MODIFY

… it blows up. Suggestion: trap/detect the following error with "transition 'accept' does not have any old/new states defined."

Traceback (most recent call last):
  File "/home/srl/src/trac/trac/web/main.py", line 512, in _dispatch_request
    dispatcher.dispatch(req)
  File "/home/srl/src/trac/trac/web/main.py", line 198, in dispatch
    chosen_handler)
  File "/home/srl/src/trac/trac/web/main.py", line 345, in _pre_process_request
    chosen_handler = filter_.pre_process_request(req, chosen_handler)
  File "/home/srl/ws/cldr-stuff/trac/IcuCodeToolsPlugin/icucodetools/review.py", line 46, in pre_process_request
    if 'ICUREVIEW_VIEW' not in req.perm:
  File "/home/srl/src/trac/trac/perm.py", line 552, in has_permission
    return self._has_permission(action, resource)
  File "/home/srl/src/trac/trac/perm.py", line 566, in _has_permission
    check_permission(action, perm.username, resource, perm)
  File "/home/srl/src/trac/trac/perm.py", line 453, in check_permission
    perm)
  File "/home/srl/src/trac/trac/perm.py", line 286, in check_permission
    get_user_permissions(username)
  File "/home/srl/src/trac/trac/perm.py", line 358, in get_user_permissions
    for requestor in self.requestors:
  File "/home/srl/src/trac/trac/core.py", line 70, in extensions
    return filter(None, [component.compmgr[cls] for cls in extensions])
  File "/home/srl/src/trac/trac/core.py", line 205, in __getitem__
    component = cls(self)
  File "/home/srl/src/trac/trac/core.py", line 111, in maybe_init
    init(self)
  File "/home/srl/src/trac/trac/ticket/api.py", line 218, in __init__
    [c.__class__.__name__ for c in self.action_controllers])
  File "/home/srl/src/trac/trac/config.py", line 652, in __get__
    for impl in self.xtnpt.extensions(instance):
  File "/home/srl/src/trac/trac/core.py", line 70, in extensions
    return filter(None, [component.compmgr[cls] for cls in extensions])
  File "/home/srl/src/trac/trac/core.py", line 205, in __getitem__
    component = cls(self)
  File "/home/srl/src/trac/trac/core.py", line 111, in maybe_init
    init(self)
  File "/home/srl/src/trac/trac/ticket/default_workflow.py", line 109, in __init__
    self.actions = get_workflow_config(self.config)
  File "/home/srl/src/trac/trac/ticket/default_workflow.py", line 85, in get_workflow_config
    actions = parse_workflow_config(raw_actions)
  File "/home/srl/src/trac/trac/ticket/default_workflow.py", line 77, in parse_workflow_config
    attributes['oldstates'].split(',')]
KeyError: 'oldstates'

[Remy Blank]

Good idea, and should even be relatively simple to implement.

[Thijs Triemstra]

against 0.12-stable r10371

[Thijs Triemstra]

attached patch prints out an error instead, visible in the web ui to the user:

Transition 'accept' in ticket workflow does not have any states defined.

[Remy Blank]

Thanks for the patch, I'll apply it.

[Remy Blank]

I ended up fixing this differently in [10376]. Personally, I dislike applications that stop working when I do a small typo in a configuration file. Instead, we now log warnings for invalid workflow actions when parsing the workflow, and just ignore those actions.

We could also display a warning for TRAC_ADMIN on every page (and I actually had a patch that did just that), but we should rather think about a more general way to display configuration errors to the administrator.