Catch errors thrown by manifest providers

[harrysarson]

Distribution objects have a list of manifest providers (both release and source) and will try each in turn until one returns non-null. However, the providers all raise exceptions rather than returning null which means the Distribution object fails to try more than one.

Typically code uses CachedManifestProvider (and
CachedSourceManifestProvider) which has this try/except logic so the looping over manifest providers works as intended.

This commit copies the try/except logic from these manifest providers directly into the methods on the Distribution class.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 5.26316% with 18 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@9e67766). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
src/rosdistro/distribution.py	5.26%	18 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #196   +/-   ##
=========================================
  Coverage          ?   39.63%           
=========================================
  Files             ?       51           
  Lines             ?     3272           
  Branches          ?      661           
=========================================
  Hits              ?     1297           
  Misses            ?     1799           
  Partials          ?      176           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tfoote]

Thanks for digging into this. It looks reasonable. It seems like we might have had a behavior evolution that changed but wasn't resolved for all the use cases.

If we're considering that this was supposed to degrade gracefully and skip providers that aren't present. Why did you add the exception rethrow versus returning none. The CachedManifestProvider doesn't use that.

rosdistro/src/rosdistro/manifest_provider/cache.py

Lines 84 to 91 in fad8d9f

	except Exception as e:
	# pass and try next manifest provider
	logger.debug('Skipped "%s()": %s' % (mp.__name__, e))
	if package_xml is None:
	return None
	# populate the cache
	self._distribution_cache.release_package_xmls[pkg_name] = package_xml
	return package_xml

[harrysarson]

Why did you add the exception rethrow versus returning none. The CachedManifestProvider doesn't use that.

I was trying to minimise the change in behaviour introduced by my patch. I thought some code might be relying on get_release_package_xml throwing an error so wanted to not change that.

Very happy to switch this to return None though.

---

Looks like the CI failure is a network error: FAILED test/test_index.py::test_get_index_from_http_with_query_parameters - urllib.error.URLError: <urlopen error <urlopen error timed out> (http://localhost:9876/index_v3.yaml?raw&at=master)> would it be possible to retry it?

Is the codecov comment something I need to fix? Looking at Please [upload](https://docs.codecov.com/docs/codecov-uploader) report for BASE (master@fad8d9f). [Learn more](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ros-infrastructure#section-missing-base-commit) about missing BASE report. Maybe CI needs to run on the master branch?