[fix] Doesn't start on Windows

CroniD commented 1 year ago

Describe the bug

Node.js version: 16.x LTS

OS version: Windows 10

Description:

Using one of the following options for root

Path.resolve('src/jobs')
Path.join(__dirname, 'jobs') leads to an error.

(also the default doesn't work)

Actual behavior

(I've cut logger and root a bit)

BREE 7784: config {
  logger: ...,
  root: 'C:\\work\\...\\src\\jobs',
  silenceRootCheckError: false,
  doRootCheck: true,
  removeCompleted: false,
  timeout: 0,
  interval: 0,
  timezone: 'local',
  jobs: [],
  hasSeconds: false,
  cronValidate: {},
  closeWorkerAfterMs: 0,
  defaultRootIndex: 'index.js',
  defaultExtension: 'js',
  acceptedExtensions: [ '.js', '.mjs' ],
  worker: {},
  errorHandler: null,
  workerMessageHandler: null,
  outputWorkerMetadata: false
}
BREE 7784: jobs []
BREE 7784: start undefined
BREE 7784: init
BREE 7784: timeout 0
BREE 7784: interval 0
BREE 7784: root C:\work\...\src\jobs
BREE 7784: doRootCheck true
BREE 7784: jobs []
BREE 7784: importPath C:\work\...\src\jobs\index.js
BREE 7784: Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file, data are supported by the default ESM loader. On Windows, absolute paths must be valid file:// URLs. Received protocol 'c:'
    at new NodeError (node:internal/errors:372:5)
    at throwIfUnsupportedURLScheme (node:internal/modules/esm/resolve:1120:11)
    at defaultResolve (node:internal/modules/esm/resolve:1200:3)
    at ESMLoader.resolve (node:internal/modules/esm/loader:580:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:294:18)
    at ESMLoader.import (node:internal/modules/esm/loader:380:22)
    at importModuleDynamically (node:internal/modules/cjs/loader:1043:29)
    at importModuleDynamicallyWrapper (node:internal/vm/module:437:21)
    at importModuleDynamically (node:vm:381:46)
    at importModuleDynamicallyCallback (node:internal/process/esm_loader:35:14)

Expected behavior

It should work.

Code to reproduce

const bree = new Bree({
    root: Path.resolve('src/jobs')
});
await bree.start();

Checklist

[x] I have searched through GitHub issues for similar issues.
[x] I have completely read through the README and documentation.
[x] I have tested my code with the latest version of Node.js and this package and confirmed it is still not working. (Note: latest LTS)

Analysis

Bree uses the import() function, which accepts URLs, but root is given as a path. Doesn't matter for unix-like systems, but it does on windows. Using a file-URL for root (e.g. Url.pathToFileURL(Path.join(__dirname, 'jobs')).toString()) stops at https://github.com/breejs/bree/blob/master/src/index.js#L209 because stat expects a path, if given as string, not a file-URL as string.

Error looks like

BREE 9112: config {
  logger: ...,
  root: 'file:///C:/work/.../src/jobs',
  silenceRootCheckError: false,
  doRootCheck: true,
  removeCompleted: false,
  timeout: 0,
  interval: 0,
  timezone: 'local',
  jobs: [],
  hasSeconds: false,
  cronValidate: {},
  closeWorkerAfterMs: 0,
  defaultRootIndex: 'index.js',
  defaultExtension: 'js',
  acceptedExtensions: [ '.js', '.mjs' ],
  worker: {},
  errorHandler: null,
  workerMessageHandler: null,
  outputWorkerMetadata: false
}
BREE 9112: jobs []
BREE 9112: start undefined
BREE 9112: init
node:internal/process/promises:279
            triggerUncaughtException(err, true /* fromPromise */);
            ^

[Error: ENOENT: no such file or directory, stat 'C:\work\...\file:\C:\work\...\src\jobs'] {
  errno: -4058,
  code: 'ENOENT',
  syscall: 'stat',
  path: 'C:\\work\\...\\file:\\C:\\work\\...\\src\\jobs'
}

I guess, if the given path is always transformed to a URL via Url.pathToFileURL(...).toString() before passing it to import(...) that might solve the issue, but I don't know if this will work out for unix-like systems as well.

CroniD commented 1 year ago

Update: Also doesn't work with NodeJS 18.12.x (LTS). Problems are the same. Paths and URIs getting mixed up, which causes errors, when running on Windows. :(

titanism commented 1 year ago

PR welcome if you want to submit a fix and also tests for Windows specifically

CroniD commented 1 year ago

PR welcome if you want to submit a fix and also tests for Windows specifically

Done. :)

titanism commented 1 year ago

v9.1.3 released to npm, please try again with this version! thank you for the PR.

https://github.com/breejs/bree/releases/tag/v9.1.3

CroniD commented 1 year ago

@titanism Thank you. It works. :)

breejs / bree