check storage path is plausible on startup

[omaskery]

Feature request

On start-up, with the default storage backend, Verdaccio should check that the configured storage folder exists and emit a warning/error log message if it is absent.

I understand that this may not apply to all storage mechanisms, but it could likely be done for the default storage mechanism used by the published docker containers.

Observed behaviour

If Verdaccio is run with an invalid storage path in its configuration it will start without warning/error, but when you try to publish a package you will receive a 404 response with the error "no such package is available".

Expected behaviour

If Verdaccio is run with an invalid storage path in its configuration it will emit a warning/error on startup and, if you try to publish a package, it would be nice if the resulting error was "unable to access storage" (or similar).

Reason

I made a very embarrassing mistake yesterday of spending most of a working day trying to debug the 404 "no such package is available" error when trying to publish to a Verdaccio, using docker, offline. I tried many things to fix it, but "no such package is available" never caused me to think of checking the storage path.
Making this get checked explicitly on startup could help prevent wasted time diagnosing faults for others in the future.

My environment

Version: docker image verdaccio/verdaccio:3.0.0-beta.7
Environment: docker (version 1.8 😱) using bind mounts (not volumes, see discussion on verdaccio/monorepo#336)

Config.yaml

example of invalid YAML storage configuration when using docker:

storage: /verdacio/

example of correct YAML storage configuration when usign docker:

storage: /verdaccio/

[omaskery]

I am new to javascript/node/npm, so forgive me if this is naive, but I have briefly investigated the verdaccio codebase. I now realise that the default storage is, I believe, in the verdaccio/localstorage project. Specifically, a check for whether the config.storage variable is a valid folder could be done in the constructor for `LocalDatabase'?

  constructor(config: Config, logger: Logger) {
    this.config = config;
    this.path = this._buildStoragePath(config);
    this.logger = logger;
+   // could we use this.logger to emit a warning here,
+   // if config.storage is not a valid directory?
    this.locked = false;
    this.data = this._fetchLocalPackages();
    this._sync();
  }

I would offer to experiment & submit a pull request but I am relatively inexperienced in this area and don't have an adequate development environment set up!

[juanpicado]

The problem is .... check whether the config.storage is not valid is an async execution, you cannot do it in the constructor. We need to find a better way. I'll think in the meantime if you find a reliable way to do it, we can discuss it.

[juanpicado]

And since recently config.storage is not mandatory anymore due to #673 . So, yes it has to be handled by the plugin itself.

[DanielRuf]

Can we not use some *Sync methods here?
What could be the needed solutions that we can implement to catch this issue?

[juanpicado]

Yes, we might deal with it, we might pay a penalty in performance startup. But I guess is doable. We have other sync methods in this plugin I guess.