PeterBooker / wpds

Slurps the latest stable version of every Plugin and/or Theme in the WordPress Directory.
GNU General Public License v2.0
20 stars 4 forks source link

wpds update themes hangs #3

Open pattonwebz opened 6 years ago

pattonwebz commented 6 years ago

When running the update command for themes wpds update themes wpds hangs when the progress bar shows up.

image This bar remains at this point never ticking up past 0.

What other info should I provide to help diagnose the issue?

PeterBooker commented 6 years ago

Thank you for reporting the issue, I have seen this once before (thanks Clorith) but am unsure how to reproduce it. It appears to be caused by Goroutines getting stuck while downloading archives and never moving onto the code which updates the progress bar.

This was one of several issues I ran into which led me to explore a centralised web service that allows anyone to run searches, but I am currently stuck working on regex compatible indexing/searching. It seems like that is going to take a lot longer than I had hoped though, so I will make some updates to WPDS first.

If I need some help debugging this issue I will let you know. Hopefully I can resolve it by adding a context to each Goroutine and ensuring they timeout after a reasonable time (60secs perhaps?). I will also see if I can add some more feedback to potentially identify why they are getting stuck.

I will let you know once I have updated. I have a few other changes (to the same code) part progressed so it might take a couple of days.

pattonwebz commented 6 years ago

Clorith was also the person who told me about this executable. No worries on taking a couple of days looking into things, I am in no rush. Let me know if I can help in any way.

I am not too familiar with GO which is why I have not been able to find and fix the problem on my own. My initial thoughts were it was trying to download with svn and failing for a reason that may be external to the code - I noticed it should attempt to fall back to http if svn isn't available.

What if SVN package appeared to be available but it fails somehow and returns a non zero exit? Would the executable account for that, consider it as a failure then fallback to http? There's a chance the SVN package on this machine is a non-standard install so it may result in unexpected output.

Like I said I am not familiar with GO so is hard for me to know how to farther debug it but I'm around should you need me.

PeterBooker commented 6 years ago

@pattonwebz Sorry for the delay. A close family member died shortly after my last reply, so I have been unexpectedly busy.

The SVN/HTTP fallback should not effect this, as it is only used to fetch the list of plugins/themes to be downloaded and once the progress bar is visible it is past that point. You have made me realise that issues with the local svn client installed could be a future source of problems, as I am only checking that an svn command is available, not that it works. I have left a TODO to explore that further.

I have been considering this problem and I am not particularly content with just adding a timeout to Goroutines as this does not solve (or even identify) the problem. From reviewing the code I have two main suspicions. First, that it was possible for the Goroutines to error unexpectedly (in a way that is not handled) which could resulted in the WaitGroup and/or Progress Bar not being notified. Second, that there is an concurrency issue with the progress bar library (or my usage of it).

I am pursuing these over this weekend and hope to have an update out Sunday, if I have time I would like to get some other improvements in too. I will respond with a summary once that is done.

Thank you for reporting the problem and I would appreciate any help testing future versions. You were brave digging into the code!

PeterBooker commented 6 years ago

It looks like it is going to take me one more day to release a new version. I have completed various changes which were in progress since the last release, which has left me free to focus on the bugs. This included basic regex search functionality.

My two suspicions have not borne fruit. I have ended up changing the progress bar library just in case and using ErrGroup (instead of WaitGroup) was a no go as it does not fit the use case. I am going to dive into the standard library for net/http to see how the lifetime of HTTP requests is handled and what I might be able to learn from that tomorrow. I will also look at what I can do to improve debugging/feedback in this area, to help identify the cause in the future.

pattonwebz commented 6 years ago

Hey @PeterBooker,

I am terribly sorry to hear of your loss and I do not want to interfere with you spending time with loved ones at the moment. Do not worry about rushing this fix out for me, given the circumstances I completely understand and no need for you to apologize.

When an update with debug logging is available to test I am happy to do so. Also if you feel like any specific info reports from my system may be of use to figure out the conflict let me know which and I'll grab them for you ASAP.

PeterBooker commented 6 years ago

Thank you. It is no problem, nothing can stop coding being great fun and satisfying! It has helped me to relax and to approach normality again.

It took longer than expected again but I have just made a new release v0.6.0. I will need to make a new release early tomorrow as I immediately ran into a bug with the progress bar printing itself repeatedly if the window is not wide enough. I also want to add in some improved resilience (HTTP retries and resumes, etc).

It does however include my best effort at avoiding any more lockups. I found a few returned error values which were being ignored, which could have been causing unexpected issues. I also changed the functions marking progress to use 'defer', which means they should be called no matter what happens to the Goroutines (as long as they close). There is no guarantee these will resolve the lockup problem, but they are my best efforts currently.

It would be great if you could try this new version and let me know if you run into the problem again. If it does I have a plan B, asking the Go community.