[TW-1520] Unsuccessful hook terminates whole execution of the task command even if it is a indirect addition

[taskwarrior]

Tomas Babej on 2015-01-20T11:54:49Z says:

I would swear I thought this worked before, but I can reproduce this on 2.4.0 as well as 2.4.1. This happens with tasks spawned by recurrence:

$ task rc.debug.hooks=3
Timer Config::load (/usr/local/share/doc/task/rc/solarized-dark-256.theme) 0.000129 sec
Timer Config::load (/home/tbabej/.taskrc) 0.000876 sec
Found hook script /home/tbabej/.task/hooks/on-add-block-recurrence
Found hook script /home/tbabej/.task/hooks/on-add-date-default-time
Found hook script /home/tbabej/.task/hooks/on-modify-date-default-time
Found hook script /home/tbabej/.task/hooks/on-modify-fix-due-shift
Hooks: Calling /home/tbabej/.task/hooks/on-add-block-recurrence
Hooks: input
  {"description":"Wake up at 6.00","due":"20150117T053000Z","entry":"20150120T115630Z","imask":3,"modified":"20150113T115244Z","parent":"5ec829f0-d911-49bb-98ac-c02152c60081","project":"Habits","recur":"1days","status":"waiting","uuid":"ca6bc236-2489-4ed9-9fa1-627c8b0cb6e0","wait":"20150117T043000Z"}
Timer Hooks::execute (/home/tbabej/.task/hooks/on-add-block-recurrence) 0.045781 sec
Hooks: output
Hooks: Completed with status 1

[task next rc.debug.hooks=3]
Configuration override rc.debug.hooks:3
Exiting with unwritten changes to /home/tbabej/.task/pending.data

Where as denied addition of the new task should not terminate the report.

[taskwarrior]

Migrated metadata:

Created: 2015-01-20T11:54:49Z
Modified: 2015-02-09T07:46:02Z

[taskwarrior]

Tomas Babej on 2015-01-20T12:24:16Z says:

Working on a patch.

[taskwarrior]

Tomas Babej on 2015-01-20T12:29:21Z says:

So I re-read the design page again and it seems this is (partially) by design. Changing to 'improvement'.

There are two ways to go, as I see it: a) Make non-zero return code value terminate only the task being added/modified b) Introduce a new return code value which will be interpreted as: disregard this addition/modification only and continue

Actually this was the behaviour I was expecting from the start.

I'd value any input as to which path is more preferable, so that I do not send soon-to-be-rejected patch :)

[taskwarrior]

Paul Beckingham on 2015-01-21T01:35:43Z says:

This is deliberate behavior. on-add needs to be able to decline the added task. on-modify needs to be able to decline the changes.

[taskwarrior]

Tomas Babej on 2015-01-21T08:28:43Z says:

I am not denying that. This bug is about something else - some commands invoke multiple onAdd operations, e.g. when recurrent tasks are generated. If any of those onAdd operations is denied, the whole command is terminated, which is not the desired behaviour in my opinion.

See how the example in the bug just tried to show the 'next' report and got terminated on the grounds that a recurrent tasks, which was just spawned, has been aborted (which user has no clue about).

[taskwarrior]

Renato Alves on 2015-01-21T11:55:07Z says:

So if I get it right the issue being raised is that hook execution cannot fail without aborting the whole command.

To avoid confusion, how about including in the output something like: "Failure while executing hook. Exit code - command aborted"

As Paul stated, hooks need a way to abort the action and that is by returning a non-zero exit code. What you are asking for is the ability to fail but continue execution... tricky. For instance what to do if you have one hook that depends on the action of another hook. Say, a on-modify hook which sets a default project, and then another hook on-modify that requires this information to make a decision.

The only way I see around this is to somehow extend hook execution to one of 2 models:

• Having 'fallible/skippable' hooks. e.g. on-modify-fallible (vs on-modify), which on error would be skipped (logging the error to console). These couldn't be used to abort changes. but would allow taskwarrior to continue if an error occurs.
• Implement some form of hook dependency (think init scripts?). Much like a tree, if one fails depending hooks would cascade the failure.

The last variant is quite possibly a feature for 2.8.0 or something like that.

[taskwarrior]

Tomas Babej on 2015-01-21T12:04:30Z says:

I am not asking for ability to continue the addition/modification of the same task, but only to continue execution of the command that triggered the action.

Having reports as "task list" being aborted due to (expected) failure of hooks applied on tasks generated by recurrence is wrong.

[taskwarrior]

Paul Beckingham on 2015-01-21T14:00:33Z says:

The part that I'm not understanding here is why your script is exiting with non-zero, when your hook is processing a task you aren't wanting to reject?

[taskwarrior]

Tomas Babej on 2015-01-21T14:17:12Z says:

I want to reject the task, therefore I exited with non-zero.

I don't want to terminate the report command.

[taskwarrior]

Tomas Babej on 2015-01-21T14:48:44Z says:

To recap and stress the imporant parts:

I understand the termination of the processing is being done when a hook exits with non-zero status.

This is totally ok (add command):

$ task add something invalid
<hook exits with 1>
<processing terminates>

This is not (list command):

$ task list
<recurrent task is generated, since no "task" command has been run for a while>
<task is invalid, hook exits with 1>
<processing terminates>

In the second example, I have not been shown my report. Which is what I wanted. I, as a user, do not care about the tasks being added in the background.