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

Replies get stuck in pending state #825

Closed
eloquence opened this issue Feb 26, 2020 · 6 comments · Fixed by #831
Closed

Replies get stuck in pending state #825

eloquence opened this issue Feb 26, 2020 · 6 comments · Fixed by #831
Labels
bug Something isn't working release blocker

Comments

@eloquence
Copy link
Member

eloquence commented Feb 26, 2020

Currently unable to send replies in 20200225-060103 nightly in Qubes w/ SuperFun test instance. All replies end up stuck in pending state forever. Seeing nothing of note in client log or proxy log. Syncs & downloads work just fine.

@eloquence eloquence added bug Something isn't working needs/reproducing labels Feb 26, 2020
@eloquence
Copy link
Member Author

eloquence commented Feb 26, 2020

Wiped the client DB and restarted the client. All the replies that seemed to be stuck did in fact send! But subsequent replies again stay in "pending" state forever (while. perhaps actually sendg)

@ninavizz
Copy link
Member

^ I experienced this, too, tonight... but I'm not fancy like Erik, and can't wipe the db.

I did it with 2 sources; composed 2 replies for one, and 1 for the other. Saw the Keyring toast mssg flash, then waited. Then waited. Then waited. Then quit the client, restarted, and they showed as "sent."

@redshiftzero redshiftzero added this to the 0.2.0beta milestone Feb 26, 2020
@redshiftzero redshiftzero added this to Current Sprint (2/20-3/4) in SecureDrop Team Board Feb 26, 2020
@sssoleileraaa
Copy link
Contributor

I am able to repro. I don't see this on master when running the unpackaged client. Since it looks like a UI issue I wonder if it's related to #633

@redshiftzero
Copy link
Contributor

Multiple people can reproduce this in Qubes, but only with the packaged code so far. I reverted to a much earlier version of securedrop-client and still see this problem, which is very odd. It can't be the SDK because testing latest master in Qubes using the RPC service, replies confirm as sent. It isn't an Apparmor denial afaict since I disabled the apparmor profile and still see the same behavior.

@eloquence
Copy link
Member Author

eloquence commented Mar 1, 2020

Still observed in latest nightly (20200229) but only in packaged build; running client master in sd-app works fine. I've attempted to manually apply the change in #831 to /opt/venvs/securedrop-client/lib/python3.7/site-packages/securedrop-client/gui but it does not appear to resolve the issue. Logs in DEBUG mode (regardless of whether the patch is applied) indicate reply success, but the UI is not updated:

Feb 29 18:04:40 localhost 2020-02-29 18:04:40,590 - securedrop_client.queue:236(enqueue) DEBUG: Added SendReplyJob to main queue
Feb 29 18:04:42 localhost 2020-02-29 18:04:42,436 - securedrop_client.logic:806(on_reply_success) DEBUG: cfcc4677-80b8-4388-932d-56999150e127 sent successfully

SecureDrop Team Board automation moved this from Current Sprint (2/20-3/4) to Done Mar 3, 2020
@rmol
Copy link
Contributor

rmol commented Mar 3, 2020

A post-mortem investigation to clarify the root cause of this problem was requested.

Our working theory was that something in recent updates to the Debian Qt5 packages broke the application of the success stylesheet to ReplyWidget's children. Specifically, in 5.11.3+dfsg1-1+deb10u2, this upstream patch was included, switching from QObject.findChildren to just children(). The former's recursive, the latter not, so I thought this might cause the stylesheet change to not be reflected down through SpeechBubble's hierarchy.

To confirm, in my Debian 10 sd-dev I:

  • ran apt source libqt5core5a
  • reverted debian/patches/repolish_run_on_direct_children.diff, removed it from debian/patches/series
  • ran dpkg-buildpackage -uc -us in the resulting qtbase-opensource-src-5.11.3+dfsg1 directory to build the 51 .debs (takes 1.25 hours, not recommended)
  • installed them all in sd-dev
  • created a virtualenv with --system-site-packages as we do when building the securedrop-client .deb, and pip installed only the production requirements
  • checked out securedrop-client at 35938694c2309aab5313bed0f414763a0329fa26 -- before I changed the way stylesheets are applied to ReplyWidget's children, so it was still just applying the stylesheet to ReplyWidget itself
  • ran the client with the dev server, and replies now worked -- they were styled as expected on success.

So I believe that proves that it's that patch from the u2 revision that broke the cascade for subclasses of QWidget. That patch came from upstream. Unless they revert the behavior, we're still going to have to work around it from this point on. I got the distinct impression that stylesheets are held in less regard among Qt developers than the docs would suggest. Some widgets only support a limited set of properties, and I saw it suggested in multiple places that you should really just be using the native Qt methods for styling widgets. That's probably a safer policy for us going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working release blocker
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants