SendReplyJob failures

[sssoleileraaa]

Description

When sending a reply fails, the follow error occurs:

  File "/home/user/securedrop-client/securedrop_client/logic.py", line 628, in on_reply_failure
    self.reply_failed.emit(reply_uuid)
TypeError: Controller.reply_failed[str].emit(): argument 1 has unexpected type 'TypeError'

Somewhere we missed this in our testing. When an exception occurs, SendReplyJob emits a signal carry with it the exception that occurred. This signal is handled by Controller.on_reply_failure which expects a uuid string.

[sssoleileraaa]

Also related (might need to be a separate issue), the SendReplyJob fails on Qubes.

[sssoleileraaa]

The exception thrown by subprocess.check_call when it tries to convert NoneType to str implicitly. This only happens on Qubes because source.fingerprint is returning None.

[eloquence]

(Pulled into current sprint as it impacts other ongoing work.)

[redshiftzero]

(current working theory on this is that the servers used for testing do not have freedomofpress/securedrop#4436 applied)

[sssoleileraaa]

Confirmed. The reason source.fingerprint was returning None on Qubes was because I was testing against staging servers that were running 0.13.0 instead of 0.13.1, which started exposing the source fingerprint (see link @redshiftzero provided above).

I'm going to keep this issue open until we merge a fix for the ValueError for when we have legit exceptions during a SendReplyJob. Right now we don't actually do anything with the reply_uuid other than log, so we could continue just retruning exceptions from ApiJob and remove the expectation of a reply_uuid. I'm in favor of that for now, but let me know if you disagree!

[redshiftzero]

It looks like the issue here is that we're emitting exceptions that are useless to the rest of the UI because it doesn't know which UUID these failures correspond to. While we could keep the exception emission (e.g. this might actually be useful to preserve for a future where we want the UI corresponding to an individual UUID element to update based on the exception that was updated), the failure slot needs to at least be getting the UUID of the item the job corresponds to. We need to make this change in the base ApiJob to emit the UUID.

[sssoleileraaa]

Per discussion with @redshiftzero:

We need to implement a solution that will allow us to emit reply_uuids to the GUI when there is a failure so that the GUI can show to the user that the reply send failed. Right now ApiJobs require that a derived class raise Exceptions that it will then emit, but this will need to be modified to accommodate the failure case where we need more information than just the exception.

[sssoleileraaa]

agreed! :)

right now we're just logging from the ReplyWidget, but after discussing how we eventually want to do more than log, it makes sense to keep this code in place and instead change the ApiJob interface