Add download timeouts for PDSC entries

[danieldegrasse]

Add a timeout of 5 seconds on downloads of PDSC entries, and log an error to stdout. This way broken PDSC entries do not hang the pyocd pack update command

Without this change:

$ pyocd pack clean
$ pyocd pack update
0000154 I Updating pack index... [pack_cmd]
Downloading descriptors (1611/1631) # command hangs here, does not complete

With this change:

$ pyocd pack clean
$ pyocd pack update
0000154 I Updating pack index... [pack_cmd]
17:44:58 [ERROR] Download of https://www.nsing.com.sg/uploadfile/file/pack/NSING.N32G05x_DFP.pdsc failed: Response code in invalid range: 500
17:44:58 [ERROR] Download of https://www.nsing.com.sg/uploadfile/file/pack/NSING.N32G030_DFP.pdsc failed: Response code in invalid range: 500
17:45:01 [ERROR] Download of http://www.ixys.com/Zilog/packs/Zilog.ZNEO32_DFP.pdsc failed: Response code in invalid range: 404
17:45:03 [ERROR] Download of https://www.abov.co.kr/data/mds/PACK/ABOV.A33G53x_Series.pdsc failed: error sending request for url (https://www.abov.co.kr/data/mds/PACK/ABOV.A33G53x_Series.pdsc)
17:45:03 [ERROR] Download of http://www.mcu.com.cn/Cmsemicon.BAT32G135-A.pdsc failed: Response code in invalid range: 404
17:45:03 [ERROR] Download of https://www.geehy.com/uploads/tool/Geehy.G32R5xx_DFP.pdsc failed: error sending request for url (https://www.geehy.com/uploads/tool/Geehy.G32R5xx_DFP.pdsc)
17:45:04 [ERROR] Download of http://www.qorvo.com/extra/keil_pack/Active-Semi.PAC52XX.pdsc failed: Response code in invalid range: 403
17:45:04 [ERROR] Download of http://www.qorvo.com/extra/keil_pack/Active-Semi.PAC55XX.pdsc failed: Response code in invalid range: 403
17:45:05 [ERROR] Download of http://download.analog.com/tools/EZBoards/ADSP-SC83x/Releases/AnalogDevices.ADSP-SC83x_FreeRTOS-OSAL.pdsc failed: error sending request for url (http://download.analog.com/tools/EZBoards/ADSP-SC83x/Releases/AnalogDevices.ADSP-SC83x_FreeRTOS-OSAL.pdsc)
17:45:06 [ERROR] Download of https://www.geehy.com/uploads/tool/Geehy.APM32F4xx_DFP.pdsc failed: error sending request for url (https://www.geehy.com/uploads/tool/Geehy.APM32F4xx_DFP.pdsc)
17:45:06 [ERROR] Download of https://www.geehy.com/uploads/tool/Geehy.APM32E1xx_DFP.pdsc failed: error sending request for url (https://www.geehy.com/uploads/tool/Geehy.APM32E1xx_DFP.pdsc)
17:45:06 [ERROR] Download of https://www.geehy.com/uploads/tool/Geehy.APM32F1xx_DFP.pdsc failed: error sending request for url (https://www.geehy.com/uploads/tool/Geehy.APM32F1xx_DFP.pdsc)
17:45:06 [ERROR] Download of https://www.geehy.com/uploads/tool/Geehy.APM32F0xx_DFP.pdsc failed: error sending request for url (https://www.geehy.com/uploads/tool/Geehy.APM32F0xx_DFP.pdsc)
17:45:06 [ERROR] Download of https://www.geehy.com/uploads/tool/Geehy.APM32F00x_DFP.pdsc failed: error sending request for url (https://www.geehy.com/uploads/tool/Geehy.APM32F00x_DFP.pdsc)
17:45:06 [ERROR] Download of https://ftp.willert.de/downloads/Keil/Pack/RXF/SodiusWillert.RXF.pdsc failed: Response code in invalid range: 404
17:45:07 [ERROR] Download of http://download.analog.com/tools/EZBoards/COG_4050/Releases/AnalogDevices.EV-COG-AD4050LZ_BSP.pdsc failed: error sending request for url (http://download.analog.com/tools/EZBoards/COG_4050/Releases/AnalogDevices.EV-COG-AD4050LZ_BSP.pdsc)
17:45:07 [ERROR] Download of http://download.analog.com/tools/EZBoards/COG_AD3029/Releases/AnalogDevices.EV-COG-AD3029LZ_BSP.pdsc failed: error sending request for url (https://download.analog.com/tools/EZBoards/COG_AD3029/Releases/AnalogDevices.EV-COG-AD3029LZ_BSP.pdsc)
17:45:07 [ERROR] Download of http://download.analog.com/tools/EZBoards/CM302x/Releases/AnalogDevices.ADuCM302x_DFP.pdsc failed: error sending request for url (http://download.analog.com/tools/EZBoards/CM302x/Releases/AnalogDevices.ADuCM302x_DFP.pdsc)
17:45:08 [ERROR] Download of https://www.geehy.com/uploads/tool/Geehy.APM32A4xx_DFP.pdsc failed: error sending request for url (https://www.geehy.com/uploads/tool/Geehy.APM32A4xx_DFP.pdsc)
17:45:09 [ERROR] Download of http://www.advsolned.com/armpack/ASN.Filter_Designer.pdsc failed: Response code in invalid range: 403
17:45:10 [ERROR] Download of https://geehy.com/uploads/tool/APEXMIC.APM32F1xx_DFP.pdsc failed: error sending request for url (https://geehy.com/uploads/tool/APEXMIC.APM32F1xx_DFP.pdsc)
17:45:11 [ERROR] Download of https://www.geehy.com/uploads/tool/Geehy.APM32S1xx_DFP.pdsc failed: error sending request for url (https://www.geehy.com/uploads/tool/Geehy.APM32S1xx_DFP.pdsc)
17:45:11 [ERROR] Download of https://www.geehy.com/uploads/tool/Geehy.APM32A0xx_DFP.pdsc failed: error sending request for url (https://www.geehy.com/uploads/tool/Geehy.APM32A0xx_DFP.pdsc)
17:45:11 [ERROR] Download of https://www.geehy.com/uploads/tool/Geehy.APM32A10x_DFP.pdsc failed: error sending request for url (https://www.geehy.com/uploads/tool/Geehy.APM32A10x_DFP.pdsc)
17:45:11 [ERROR] Download of https://www.geehy.com/uploads/tool/Geehy.APM32F035_DFP.pdsc failed: error sending request for url (https://www.geehy.com/uploads/tool/Geehy.APM32F035_DFP.pdsc)
Downloading descriptors (1631/1631)

Command completes and I can use the packs that downloaded correctly

[maxgerhardt-phy]

+1! With this issue being present, it breaks some workflows like building containers that do a pyocd pack update.

[dhommerOXOS]

+1

[gbarkadiusz]

Who has the access to merge this pr?

[mathias-arm]

Who has the access to merge this pr?

I have. Having feedback about the fact people are encountering the same problem is useful. What is more useful is feedback about testing the pull request. I can only assume that emojis, +1 comments, and empty reviews are the former.

I have two concerns with the PR:

• What is the likelihood that it breaks pack updates for some people? There is an arbitrary value of 5 seconds with no reasoning for choosing this value (why not 2, 10 or 30 seconds?).
• There is no justification for changing the download failures from warning to errors. It's certainly useful when you are debugging this issue, but we have to consider if this is going to be helpful in the general case or cause confusion.

[danieldegrasse]

Who has the access to merge this pr?

I have. Having feedback about the fact people are encountering the same problem is useful. What is more useful is feedback about testing the pull request. I can only assume that emojis, +1 comments, and empty reviews are the former.

I have two concerns with the PR:

* What is the likelihood that it breaks pack updates for some people? There is an arbitrary value of 5 seconds with no reasoning for choosing this value (why not 2, 10 or 30 seconds?).

* There is no justification for changing the download failures from warning to errors. It's certainly useful when you are debugging this issue, but we have to consider if this is going to be helpful in the general case or cause confusion.

Hey- these are both fair concerns. The choice of a 5 second timeout was arbitrary, and essentially made so that the pack update would not hang for a long period when a timeout was going to occur. If you'd like a longer timeout I'm ok with that.

The reason I changed the download failure to an error was that I did not see a log emitted (in the default configuration) when using a warning. IMO users should be aware some packs are failing to download- if for nothing else than so that they can ping the vendors maintaining those packs.

[mathias-arm]

The choice of a 5 second timeout was arbitrary, and essentially made so that the pack update would not
hang for a long period when a timeout was going to occur. If you'd like a longer timeout I'm ok with that.

The timeout method you use includes both the connection time and the time it takes to transfer of the content:

      The timeout is applied from when the request starts connecting until the response body has finished.

It seems this would be very problematic as it would impact all users that don't have an Internet connection that allows them to download a .pdsc file or .pack file within 5 seconds. .pack files can be quite large, so that could in theory impact all users. Have you tried using connect_timeout?

It seems that the TCP connection timeout is 127 seconds, based on https://man7.org/linux/man-pages/man7/tcp.7.html:

       tcp_syn_retries (integer; default: 6; since Linux 2.2)
              The maximum number of times initial SYNs for an active TCP
              connection attempt will be retransmitted.  This value
              should not be higher than 255.  The default value is 6,
              which corresponds to retrying for up to approximately 127
              seconds.  Before Linux 3.7, the default value was 5, which
              (in conjunction with calculation based on other kernel
              parameters) corresponded to approximately 180 seconds.

I would be interested to see if connect_timeout works and how a value of say 15 seconds compared to 5 seconds impact the overall operation.

IMO users should be aware some packs are failing to download- if for nothing else than so that they can
ping the vendors maintaining those packs.

There are always failing downloads. Your view of users does not align with our experience. If we display error messages we are assured to confuse users (in particular the existing ones who have not seen those messages in the past) and are more likely to open an issue here than report to the vendor. If you want to see the warnings use -v.

[danieldegrasse]

I would be interested to see if connect_timeout works and how a value of say 15 seconds compared to 5 seconds impact the overall operation.

IMO users should be aware some packs are failing to download- if for nothing else than so that they can
ping the vendors maintaining those packs.

I've updated the PR to use connect_timeout with a duration on 15 seconds, and timeout with a duration of 60 seconds to set an upper bound (the documentation of ClientBuilder claims the default is to never timeout)

With this, pyocd pack update does take longer (since we need to hit the 60 second timeout for some URLs that connect but then fail)- but the downloads do timeout. Based on this, I'm convinced we need some upper bound on download timeouts.

There are always failing downloads. Your view of users does not align with our experience. If we display error messages we are assured to confuse users (in particular the existing ones who have not seen those messages in the past) and are more likely to open an issue here than report to the vendor. If you want to see the warnings use -v.

Using pyocd pack update -vvv, I don't see any warnings. I also tried with pack-manager cache devs -vvv. Maybe the verbosity isn't being passed down to the rust module? . I don't love the idea of keeping these logs at a warning (won't users want to know that something failed?)- that being said I'm much more concerned with getting the timeout added, because with the current version of cmsis-pack-manager I am unable to download packs at all.

[mathias-arm]

With this, pyocd pack update does take longer (since we need to hit the 60 second timeout for some URLs that connect but then fail)- but the downloads do timeout. Based on this, I'm convinced we need some upper bound on download timeouts.

Using timeout is not the solution because it will break downloading .pack files. Setting the read_timeout to 15 seconds seems to work. Can you check with the version I pushed?

I was wrong about the -v. I typically use the cmsis-cli when testing. The fix is not easy so we will table that for now.

[danieldegrasse]

Using timeout is not the solution because it will break downloading .pack files. Setting the read_timeout to 15 seconds seems to work. Can you check with the version I pushed?

I was wrong about the -v. I typically use the cmsis-cli when testing. The fix is not easy so we will table that for now.

Just checked- the currently pushed version with read_timeout works for me, thanks for this fix. I get the reasoning with timeout- essentially we want to make sure users with slow network connections don't timeout, right?

[mathias-arm]

I get the reasoning with timeout- essentially we want to make sure users with slow network connections don't timeout, right?

Yes. What we are concerned about is that connections or communications are progressing, not capping the time for the whole transfer to an arbitrary value. The .pdsc files are a few 100 KiB, with 2 having pushed beyond 1 MiB, that means with a timeout at 30 seconds, you could not download those files unless you could sustain 30 KiB/s to those servers. Now if we consider a .pack file of say 100 MiB, with the same timeout downloads would fail unless if bandwidth is below 3 MiB/s.

[gbarkadiusz]

Regarding this. Can we have the new code release with this fix?
v0.5.3 released on Oct 6, 2023.
Of course it can be installed manually from source, but it requires rust toolchain and so on.

Justification:
The pyocd uses latest (v.0.5.3) version https://github.com/pyocd/pyOCD/blob/90ee63a57a73ccca6da10b4ad5e3ebd5a6775e58/setup.cfg#L54

I'd like to have this fix confirmed to close this issue.
pyocd/pyOCD#1783

[mathias-arm]

@gbarkadiusz, @danieldegrasse, @maxgerhardt-phy, @orlra, @dhommerOXOS.
I prepared a [48e93d7]. If you can test the wheels built in CI, that will help speed up the release.

[danieldegrasse]

@gbarkadiusz, @danieldegrasse, @maxgerhardt-phy, @orlra, @dhommerOXOS. I prepared a [48e93d7]. If you can test the wheels built in CI, that will help speed up the release.

Verified the fix work on Mac OS Sequoia (15.5), using wheel from here: https://github.com/pyocd/cmsis-pack-manager/actions/runs/15828200318/artifacts/3384328826

[MadsAndreasen]

@gbarkadiusz, @danieldegrasse, @maxgerhardt-phy, @orlra, @dhommerOXOS. I prepared a [48e93d7]. If you can test the wheels built in CI, that will help speed up the release.

Verified on Ubuntu 25.04 x86_64

[mathias-arm]

The new version was released (https://github.com/pyocd/cmsis-pack-manager/releases/tag/v0.6.0) and published on PyPI (https://pypi.org/project/cmsis-pack-manager/0.6.0/).