Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when sending reply [Nov 14 nightly / SF instance] #622

Closed
eloquence opened this issue Nov 14, 2019 · 3 comments · Fixed by #623
Closed

Crash when sending reply [Nov 14 nightly / SF instance] #622

eloquence opened this issue Nov 14, 2019 · 3 comments · Fixed by #623
Labels
bug Something isn't working

Comments

@eloquence
Copy link
Member

When sending reply in client, client crashes. Log in .securedrop_client sayeth:

2019-11-14 15:03:59,867 - root:53(excepthook) ERROR: Unrecoverable error
Traceback (most recent call last):
  File "/opt/venvs/securedrop-client/lib/python3.5/site-packages/securedrop_client/gui/widgets.py", line 2423, in send_reply
    self.controller.send_reply(self.source.uuid, reply_uuid, reply_text)
  File "/opt/venvs/securedrop-client/lib/python3.5/site-packages/securedrop_client/logic.py", line 737, in send_reply
    journalist_id=self.user.uuid,
AttributeError: 'Controller' object has no attribute 'user'
@eloquence eloquence added the bug Something isn't working label Nov 14, 2019
@eloquence
Copy link
Member Author

eloquence commented Nov 14, 2019

(This is reproducible, independent of which source is selected, on the SF instance.)

@eloquence eloquence changed the title Crash when sending reply [Nov 14 nightly] Crash when sending reply [Nov 14 nightly / SF instance] Nov 14, 2019
@redshiftzero
Copy link
Contributor

we need #381 to help catch bugs like this pre-merge

@redshiftzero
Copy link
Contributor

ok so this bug was a result of our 'no rebase before final merge' policy, read on for the details.

When #605 was in the PR stage, #578 was merged introducing the use of the user object during the sending of replies (along with lots of other changes needed for the pending replies).

Next, we merged #605 removing the user object from the controller - since the merge of #578 didn't produce any conflicts, we introduced this bug only when we merged to master.

If we'd have rebased prior to final merge and re-tested everything (or had functional tests that would have then failed), we'd have discovered this bug.

I think going forward we should be careful to rebase/retest other pending changes after a big PR lands, but I don't want to do this across the board until we see another defect introduced due to this cause (thoughts welcome though).

Anyway for resolving the bug updating the reference from self.user.uuid to self.api.token_journalist_uuid should resolve this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants