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] Doesn't start on Windows #202

Closed
3 tasks done
CroniD opened this issue Oct 3, 2022 · 5 comments · Fixed by #207
Closed
3 tasks done

[fix] Doesn't start on Windows #202

CroniD opened this issue Oct 3, 2022 · 5 comments · Fixed by #207
Labels
bug Something isn't working

Comments

@CroniD
Copy link
Contributor

CroniD commented Oct 3, 2022

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

  • I have searched through GitHub issues for similar issues.
  • I have completely read through the README and documentation.
  • 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 CroniD added the bug Something isn't working label Oct 3, 2022
@CroniD
Copy link
Contributor Author

CroniD commented Nov 22, 2022

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
Copy link
Contributor

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

@CroniD CroniD mentioned this issue Nov 26, 2022
6 tasks
@CroniD
Copy link
Contributor Author

CroniD commented Nov 26, 2022

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

Done. :)

titanism pushed a commit that referenced this issue Nov 27, 2022
@titanism
Copy link
Contributor

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
Copy link
Contributor Author

CroniD commented Nov 27, 2022

@titanism Thank you. It works. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants