renovatebot / renovate

Home of the Renovate CLI: Cross-platform Dependency Automation by Mend.io
https://mend.io/renovate
GNU Affero General Public License v3.0
17.57k stars 2.31k forks source link

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

Closed casdevs closed 2 years ago

casdevs commented 4 years ago

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 https://github.com/renovatebot/renovate/blob/c2ebd71732e420c2bc8c80af526ccc7a644b1775/lib/config/file.ts#L18 only checking for errors of type SyntaxError and proceeding silently on all other types of errors (evaluation exceptions, fileNotFound exception etc.)

casdevs commented 4 years ago

@rarkins: thanks for accepting the bug report

But I think this issue should be prioritized, because the incomplete checks in https://github.com/renovatebot/renovate/blob/c2ebd71732e420c2bc8c80af526ccc7a644b1775/lib/config/file.ts#L18 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).

rarkins commented 4 years ago

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 commented 2 years ago

I'd like to take this one.

olegkrivtsov commented 2 years ago

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:

https://github.com/renovatebot/renovate/blob/789caadbb49e232ef74f59f5c41c1a6fbffee5c0/lib/workers/global/config/parse/file.ts#L24-L27

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 commented 2 years ago

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

viceice commented 2 years ago

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

olegkrivtsov commented 2 years ago

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

rarkins commented 2 years ago

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

olegkrivtsov commented 2 years ago

Hi @rarkins I've checked and the problem still exists. I tried to fix it in https://github.com/renovatebot/renovate/pull/13196

renovate-release commented 2 years ago

:tada: This issue has been resolved in version 31.11.5 :tada:

The release is available on:

Your semantic-release bot :package::rocket: