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(datasource/repology): throw if package/repo not found #31093

Closed
wants to merge 2 commits into from

Conversation

kayman-mk
Copy link

Changes

This PR adds a warning to the log file if the Repology datasource is not able to find the package and/or repository. It was a debug message before.

Context

In case package/repository are not found it needs to visible in the daily work with Renovate as these packages are never updated due to a wrong setup by the user. Usually the logs are set to info or even warn as you don't wan't to see debug messages on a regular basis.

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

);

return undefined;
throw new Error(`Repository or package not found on Repology: ${repoName}/${pkgName}`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
throw new Error(`Repository or package not found on Repology: ${repoName}/${pkgName}`);
throw err;

Copy link
Author

Choose a reason for hiding this comment

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

But this does not work as there is no err.

TS2304: Cannot find name err

Copy link
Collaborator

Choose a reason for hiding this comment

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

How is this code reached and for what reasons?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I looked and there's very few manually thrown errors in datasources. Ideally we shouldn't invent a new type of error here

Copy link
Author

@kayman-mk kayman-mk Aug 31, 2024

Choose a reason for hiding this comment

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

Before there was a logger.debug which I changed to logger.warn to make the problem visible in the logs. See above. May be switch back to the first solution?

@viceice viceice changed the title chore: use logger.warn if package/repo not found fix(datasource/repology): throw if package/repo not found Aug 30, 2024
@kayman-mk kayman-mk requested a review from rarkins August 31, 2024 07:32
@rarkins rarkins added the auto:discussion-first This PR needs to be preceded by a GitHub Discussion label Sep 2, 2024
Copy link
Contributor

github-actions bot commented Sep 2, 2024

Please create a GitHub Discussion before continuing with this PR.

Thank you for your PR, but we need to discuss the requirements and implementation first.

The maintainers believe that there is a lack of - or misalignment of - requirements about this PR. We need to discuss the requirements and implementation first so that you don't write code that won't be merged.

This PR will be closed for now to avoid confusion, but you can reopen it after the discussion has been resolved.

Thanks, the Renovate team

@github-actions github-actions bot closed this Sep 2, 2024
@rarkins
Copy link
Collaborator

rarkins commented Sep 2, 2024

Sorry, let's have a discussion about this first, because I don't think this is an obvious change or repology-specific.

null is a valid result, Repology probably isn't the only datasource which can return it and shouldn't have unique handling of it

abstract getReleases(
getReleasesConfig: GetReleasesConfig,
): Promise<ReleaseResult | null>;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto:discussion-first This PR needs to be preceded by a GitHub Discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants