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

renovate silently ignores config if RENOVATE_CONFIG_FILE points to a non-existing file or config.js throws on evaluation #7620

Closed
casdevs opened this issue Oct 31, 2020 · 10 comments · Fixed by #13196
Assignees
Labels
core:config Related to config capabilities and presets help wanted Help is needed or welcomed on this issue priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:bug Bug fix of existing functionality

Comments

@casdevs
Copy link
Contributor

casdevs commented Oct 31, 2020

What Renovate type, platform and version are you using?

latest renovate version on self-hosted GitLab instance

Describe the bug

When running renovate-config-validator on the following config.js file (which throws on evaluation if CI_API_V4_URL is undefined):

module.exports = {
    platform: 'gitlab',
    ...
    endpoint: CI_API_V4_URL.replace(...)
    ...
};

it incorrectly prints

Validating config.js
OK

instead of failing and/or re-throwing the exception thrown by the (incorrect) config.js file.

The same happens if we set the RENOVATE_CONFIG_FILE env variable to a non-existing filename.
I would expect that renovate-config-validator also throws or fails in this case.

This is because of

if (err instanceof SyntaxError) {

only checking for errors of type SyntaxError and proceeding silently on all other types of errors (evaluation exceptions, fileNotFound exception etc.)

@rarkins rarkins added type:bug Bug fix of existing functionality priority-4-low Low priority, unlikely to be done unless it becomes important to more people labels Nov 1, 2020
@casdevs
Copy link
Contributor Author

casdevs commented Nov 2, 2020

@rarkins: thanks for accepting the bug report

But I think this issue should be prioritized, because the incomplete checks in

if (err instanceof SyntaxError) {
are also used by renovate itself, meaning that at least in both mentioned cases (if RENOVATE_CONFIG_FILE points to a non-existing file or the configured config.js file throws on evaluation) renovate silently ignores the config and proceeds only with what is left (default config, project-specific config etc.)

This may lead to configured functionality not working and larger portions of config not being applied as expected, without further notice (at least if no debug logging is enabled).

@casdevs casdevs changed the title renovate-config-validator silently fails if config.js evaluation throws or if RENOVATE_CONFIG_FILE points to a non-existing file renovate silently ignores if RENOVATE_CONFIG_FILE points to a non-existing file or config.js throws on evaluation Nov 2, 2020
@casdevs casdevs changed the title renovate silently ignores if RENOVATE_CONFIG_FILE points to a non-existing file or config.js throws on evaluation renovate silently ignores config if RENOVATE_CONFIG_FILE points to a non-existing file or config.js throws on evaluation Nov 2, 2020
@rarkins rarkins added priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others help wanted Help is needed or welcomed on this issue and removed priority-4-low Low priority, unlikely to be done unless it becomes important to more people labels Nov 2, 2020
@rarkins
Copy link
Collaborator

rarkins commented Nov 2, 2020

Alternative 1: debug and find out which type of error is being thrown, add it to the clause

Alternative 2: work out if there's a specific error for "file not found" and flip the logic so that we ignore if that's thrown but error for all others

@olegkrivtsov
Copy link
Contributor

I'd like to take this one.

@rarkins rarkins assigned olegkrivtsov and unassigned yoswein Nov 18, 2021
@olegkrivtsov
Copy link
Contributor

olegkrivtsov commented Nov 19, 2021

I checked on my Ubuntu VM, and require() throws a ReferenceError when it can't find an env var:

ReferenceError: CI_API_V4_URL is not defined

If it can't find config.ts, require() throws an Error with the code field set to MODULE_NOT_FOUND: https://stackoverflow.com/questions/13197795/handle-errors-thrown-by-require-module-in-node-js

 {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/home/renovate/lib/workers/global/config/parse/file.ts',
    '/home/renovate/lib/workers/global/config/parse/index.ts',
    '/home/renovate/lib/workers/global/index.ts',
    '/home/renovate/lib/renovate.ts'
  ]
}

So, my plan for fixing this issue:

  1. Edit lib/workers/global/config/parse/file.ts and modify this block:

} else {
// istanbul ignore next: we can ignore this
logger.debug('No config file found on disk - skipping');
}

Check if we have a ReferenceError and exit:

logger.fatal('Config file parsing error - maybe check for undefined ENV vars?');
process.exit(1);

Else check for MODULE_NOT_FOUND code, if it presents, exit:

logger.fatal('Config file seems to be missing');
process.exit(1);

Else ignore the error.

@rarkins
Copy link
Collaborator

rarkins commented Nov 19, 2021

@viceice I think this might conflict with or be made redundant by #12644, maybe should be canceled or deferred?

@viceice
Copy link
Member

viceice commented Nov 19, 2021

Yes, @olegkrivtsov Please defer this issue until we get this merged:

@viceice viceice added status:blocked Issue is blocked by another issue or external requirement and removed status:ready labels Nov 19, 2021
@olegkrivtsov
Copy link
Contributor

Hi @viceice since #12644 is merged now, can I proceed with this one?

@rarkins
Copy link
Collaborator

rarkins commented Dec 14, 2021

Yes, but maybe it's already solved - pleases verify

@viceice viceice added status:ready core:config Related to config capabilities and presets and removed status:blocked Issue is blocked by another issue or external requirement labels Dec 14, 2021
@olegkrivtsov
Copy link
Contributor

Hi @rarkins I've checked and the problem still exists. I tried to fix it in #13196

@renovate-release
Copy link
Collaborator

🎉 This issue has been resolved in version 31.11.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core:config Related to config capabilities and presets help wanted Help is needed or welcomed on this issue priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:bug Bug fix of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants