sematext / logsene-js

Log shipping library for sending logs to Sematext from JavaScript apps
https://sematext.com/cloud
Apache License 2.0
10 stars 9 forks source link

Node 18 compatibility #33

Closed yelworc closed 1 year ago

yelworc commented 2 years ago

The lib does not declare compatibility with particular Node.js versions (e.g. via engines in package.json – maybe it should?), but it doesn't seem to work with Node 18:

logsene-js on  master [!] via ⬢ v18.1.0 is 📦 v1.1.75 
➜ npm -s run test
/home/…/logsene-js/node_modules/yargs/yargs.js:1172
      else throw err
           ^

TypeError: details.family.toLowerCase is not a function
    at /home/…/logsene-js/node_modules/ip/lib/ip.js:385:39
    at Array.filter (<anonymous>)
    at /home/…/logsene-js/node_modules/ip/lib/ip.js:384:37
    at Array.map (<anonymous>)
    at ip.address (/home/…/logsene-js/node_modules/ip/lib/ip.js:379:37)
    at Object.<anonymous> (/home/…/logsene-js/index.js:25:31)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Module._load (node:internal/modules/cjs/loader:827:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/home/…/logsene-js/test/test.js:3:15)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Module._load (node:internal/modules/cjs/loader:827:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at /home/…/logsene-js/node_modules/mocha/lib/mocha.js:334:36
    at Array.forEach (<anonymous>)
    at Mocha.loadFiles (/home/…/logsene-js/node_modules/mocha/lib/mocha.js:331:14)
    at Mocha.run (/home/…/logsene-js/node_modules/mocha/lib/mocha.js:809:10)
    at exports.singleRun (/home/…/logsene-js/node_modules/mocha/lib/cli/run-helpers.js:108:16)
    at exports.runMocha (/home/…/logsene-js/node_modules/mocha/lib/cli/run-helpers.js:142:13)
    at exports.handler (/home/…/logsene-js/node_modules/mocha/lib/cli/run.js:292:3)
    at Object.runCommand (/home/…/logsene-js/node_modules/yargs/lib/command.js:242:26)
    at Object.parseArgs [as _parseArgs] (/home/…/logsene-js/node_modules/yargs/yargs.js:1113:24)
    at Object.parse (/home/…/logsene-js/node_modules/yargs/yargs.js:575:25)
    at exports.main (/home/…/logsene-js/node_modules/mocha/lib/cli/cli.js:68:6)
    at Object.<anonymous> (/home/…/logsene-js/node_modules/mocha/bin/mocha:162:29)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Module._load (node:internal/modules/cjs/loader:827:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12)
    at node:internal/main/run_main_module:17:47

Node.js v18.1.0

Updating the ip dependency (to v1.1.8) fixes the module loading issue, but there are still test failures (that don't occur with Node 16 on the same system):

  18 passing (2m)
  2 failing

  1) Accept dynamic index name function
       generates index name per document:
     Error: Timeout of 25000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/…/logsene-js/test/test.js)
      at listOnTimeout (node:internal/timers:564:17)
      at process.processTimers (node:internal/timers:507:7)

  2) Using _index from message + remove _index field from message
       generates index name per document:
     Error: Timeout of 25000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/…/logsene-js/test/test.js)
      at listOnTimeout (node:internal/timers:564:17)
      at process.processTimers (node:internal/timers:507:7)
otisg commented 2 years ago

Thanks for reporting this, @yelworc. We'll look into this, but you also seem to know your way around Node.js, so if you are able to provide a PR, that will speed things up. We can then easily and quickly provide (you with) a new release. Is this something you could do?

yelworc commented 2 years ago

Thanks for the quick response @otisg! After opening the issue, I realized that this only applies when updating the package in a project where it's already installed; otherwise, ip is installed in its latest version due to the ^ in package.json. I simply removed and re-added logsene-js in my project, and it seems to be working just fine with Node 18.

Feel free to close the issue, or leave it open for the two unit tests that are failing on Node 18 (I briefly looked into that, but I'm not sure what the right approach would be there. Maybe it's just something in my environment that breaks them).

regiluze commented 1 year ago

Tested in Node 18 and it works fine.