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

Fix NilClass error in format_version_releases for Python package details fetcher #11789

Merged
merged 1 commit into from
Mar 11, 2025

Conversation

kbukum1
Copy link
Contributor

@kbukum1 kbukum1 commented Mar 11, 2025

What are you trying to accomplish?

This PR fixes an issue where fetch_from_json_registry in PackageDetailsFetcher could return nil, leading to a Sorbet type validation error:

Dependabot::Sorbet::Runtime::InformationalError: Parameter 'releases_json': Expected type T::Hash[String, T::Array[T::Hash[String, T.untyped]]], got type NilClass

This error occurs when the registry response is unexpectedly empty or missing the releases key. The fix ensures that fetch_from_json_registry handles this case gracefully by providing a default empty hash ({}) if the response is nil.

What issues does this affect or fix?

Fixes:


Anything you want to highlight for special attention from reviewers?

  • The fix ensures that even if fetch_from_json_registry returns nil, the method does not break due to unexpected NilClass values.
  • Alternative solutions could involve handling this at a higher level, but this approach minimizes impact while fixing the issue directly at the source.

How will you know you've accomplished your goal?

  • ✅ Successfully tested that fetch_from_json_registry no longer causes Sorbet type validation errors.
  • ✅ Dependabot now properly processes package details even if releases_json is missing from the registry response.

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@kbukum1 kbukum1 marked this pull request as ready for review March 11, 2025 18:02
@kbukum1 kbukum1 requested a review from a team as a code owner March 11, 2025 18:02
@@ -268,11 +268,13 @@ def version_details_from_link(link)

sig do
params(
releases_json: T::Hash[String, T::Array[T::Hash[String, T.untyped]]]
releases_json: T.nilable(T::Hash[String, T::Array[T::Hash[String, T.untyped]]])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why make this nilable though?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue occurs because the JSON response from the registry does not include releases. The response body is either empty or contains unexpected data.

Normally, when releases information is missing from JSON, we fall back to the HTML registry, which is the standard process for Python dependencies. However, due to this Sorbet type validation error, the process fails before reaching the fallback. The root cause is that the releases parameter is being passed as nil, triggering a type mismatch in format_version_releases.

This fix ensures that when releases is missing or nil, we handle it gracefully without breaking the normal fallback mechanism.

@kbukum1 kbukum1 requested a review from abdulapopoola March 11, 2025 18:11
@kbukum1 kbukum1 merged commit a613498 into main Mar 11, 2025
70 checks passed
@kbukum1 kbukum1 deleted the kamil/fix-sorbet-typings-for-nill-parameters branch March 11, 2025 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants