Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#2028 closed Enhancement (fixed)

transmission-remote's exit code should be nonzero on failure

Reported by: gaa Owned by: adurity
Priority: Normal Milestone: 1.70
Component: Daemon Version: 1.51+
Severity: Minor Keywords: patch-needed
Cc:

Description

It is a common practice to return non-zero exit code if command execution was failed.

Test 1:

$ transmission-remote torrenthost -n $user:$passwd -a /tmp/nonexistent-file torrenthost:9091 responded: "invalid or corrupt torrent file" $ echo $? 0

Expected results:

Exit code is non-zero

Actual results:

Exit code is zero


Test 2:

$ transmission-remote torrenthost -n $user:$passwd -a tmp/nonexistent-file 10.22.0.1:9091 responded: "success" $ echo $? 0

Results:

OK, exit code is zero

Attachments (1)

remote_exitcode.patch (5.8 KB) - added by adurity 12 years ago.
Patch file for this enhancement

Download all attachments as: .zip

Change History (17)

comment:1 Changed 12 years ago by gaa

It is a common practice to return non-zero exit code if command execution was failed.


Test 1:

$ transmission-remote torrenthost -n $user:$passwd -a /tmp/nonexistent-file
torrenthost:9091 responded: "invalid or corrupt torrent file"
$ echo $?
0

Expected results:

Exit code is non-zero

Actual results:

Exit code is zero


Test 2:

$ transmission-remote torrenthost -n $user:$passwd -a tmp/nonexistent-file
10.22.0.1:9091 responded: "success"
$ echo $?
0

Results:

OK, exit code is zero

comment:2 Changed 12 years ago by charles

What is the use case for this change?

comment:3 Changed 12 years ago by gaa

I have shell-script that bound to torrent files in firefox.

reply="`transmission-remote "$HOST":"$PORT" -n "$USER":"$PASS" -a "$torrentFile"`"
echo "$reply" | grep -q success
if [ $? = 0 ]
then
    kdialog --title "$0" --msgbox "Torrent has been successfully added!"
    rm "$torrentFile"
else
    kdialog --title "$0" --error "Unable to add torrent:\n$reply"
    rm "$torrentFile"
    exit 1
fi

Corrently I need to parse command result. But it is not reliable: output string can be changed in a future releases and/or can be translated to my language (my locale is not en_US). Checking exit code is a more reliable and common practice.

If exit code will be adjusted, my script can be rewritten in the following manner:

transmission-remote "$HOST":"$PORT" -n "$USER":"$PASS" -a "$torrentFile"
if [ $? = 0 ]
then
    kdialog --title "$0" --msgbox "Torrent has been successfully added!"
    rm "$torrentFile"
else
    kdialog --title "$0" --error "Unable to add torrent:\n$reply"
    rm "$torrentFile"
    exit 1
fi

comment:4 Changed 12 years ago by charles

  • Component changed from CLI to Daemon

I guess that's reasonable.

Could you cook up a patch for remote.c?

comment:5 Changed 12 years ago by gaa

I think that implementing this enhancement will require RPC protocol enhancement because command execution result are not sent inside server's responce (only string message are sent). I does not have enough code knowledge to implement that.

comment:6 Changed 12 years ago by charles

  • Summary changed from Exit code of transmission-remote should be non-zero if operation has been failed to transmission-remote's exit code should be nonzero if the operation failed.

comment:7 Changed 12 years ago by charles

you don't need to change the RPC protocol. The response's "result" value MUST be "success" on success, so you can just test for that and return nonzero when it's not "success".

only remote.c needs to be changed for this ticket.

comment:8 Changed 12 years ago by charles

  • Keywords needs-patch added

comment:9 Changed 12 years ago by charles

gaa: are you going to patch remote.c to do this?

comment:10 Changed 12 years ago by gaa

Sorry, I am too busy now. May be on the next week.

comment:11 Changed 12 years ago by charles

  • Summary changed from transmission-remote's exit code should be nonzero if the operation failed. to transmission-remote's exit code should be nonzero on failure

comment:12 Changed 12 years ago by charles

  • Keywords patch-needed added; needs-patch removed

comment:13 Changed 12 years ago by charles

gaa: any progress on this?

Changed 12 years ago by adurity

Patch file for this enhancement

comment:14 Changed 12 years ago by adurity

  • Owner set to adurity
  • Status changed from new to assigned

I have attached a patch file that I believe satisfies this enhancement. It returns EXIT_SUCCESS when all is normal, and returns EXIT_FAILURE when something goes wrong. This mechanism is POSIX compliant and thus portable.

comment:15 Changed 12 years ago by charles

  • Resolution set to fixed
  • Status changed from assigned to closed

thanks adurity!

comment:16 Changed 12 years ago by charles

  • Milestone changed from None Set to 1.70
Note: See TracTickets for help on using tickets.