Updating npm - Githubissues

ether / etherpad-lite

Etherpad: A modern really-real-time collaborative document editor.

http://docs.etherpad.org/

Apache License 2.0

16.06k stars 2.79k forks source link

Updating npm #5812

Closed webzwo0i closed 4 months ago

webzwo0i commented 11 months ago

After trying to fix the failing admin tests for quite some time it is clear now that the problem is due to npm.

We have a number of bug reports (e.g. https://github.com/ether/etherpad-lite/issues/5643 https://github.com/ether/etherpad-lite/issues/5624 https://github.com/ether/etherpad-lite/issues/5569 ) I even diagnosed this last year, but totally forgot about it. :-( https://github.com/ether/etherpad-lite/issues/5609

During tests I noticed that in recent versions of npm packages again get deleted, if they are not stated in package.json. Because I don't have much insight into npm to solve all the problems we currently have, my approach is:

remove npm from our package.json, as we don't really use it internally (maybe a plugin does)
rebuild package-lock.json with npm v8 (which is in Node v16, our currently minimal supported version)
get rid of the symbolic link ep_etherpad-lite; instead use npm link ep_etherpad-lite in the root directory to build a package.json file and install all the dependencies in node_modules, incl. the link to ./src
store installed plugins into the package.json file in the root directory
get rid of --legacy-pper-deps and --no-save as this is not necessary anymore and/or discouraged

The two package.json files are different. The one in src only contains the dependencies of Etherpad core, while the file in the root directory essentially describes the whole project (core + plugins).

Admin tests, frontend tests and backend tests on linux already work.

We no longer need to re-install core with ./bin/installDeps.sh after installing plugins. We got some bad feedback because this behavior was counterintuitive.

As a result, only ROOT_DIR/node_modules contains packages anymore. src/node_modules is gone.

TODO:

[] find out if plugins depend on a version of npm without explicitly saying this in their package.json
investigate if fastRun still works
fix Dockerfile production vs. dev builds
npmpublish script (on npm no recent etherpad version is published since 1.8.14)
checkPlugin.js adaption
changelog entry that clearly describes the new behavior for admins and plugin devs

@JohnMcLear @SamTV12345 What do you think? This is not 100% ready, but the idea seems to work in general.

webzwo0i commented 11 months ago

Our frontend tests seem to be green, but there are some errors with log4js.

Error: 023-07-05 23:01:15.511] [ERROR] console - Failed to load hook function "/home/runner/work/***-lite/***-lite/node_modules/ep_readonly_guest:eejsBlock_permissionDenied" for plugin "ep_readonly_guest" part "ep_readonly_guest" hook set "hooks" hook "eejsBlock_permissionDenied": Error: Cannot find module 'ep_***-lite/node_modules/log4js'
Require stack:
- /home/runner/work/***-lite/***-lite/node_modules/ep_readonly_guest/index.js
- /home/runner/work/***-lite/***-lite/src/static/js/pluginfw/shared.js
- /home/runner/work/***-lite/***-lite/src/static/js/pluginfw/plugins.js
- /home/runner/work/***-lite/***-lite/src/node/server.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1072:15)
    at Module._load (node:internal/modules/cjs/loader:925:27)
    at Module.require (node:internal/modules/cjs/loader:1139:19)
    at require (node:internal/modules/helpers:121:18)
    at Object.<anonymous> (/home/runner/work/***-lite/***-lite/node_modules/ep_readonly_guest/index.js:6:16)
    at Module._compile (node:internal/modules/cjs/loader:1257:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1311:10)
    at Module.load (node:internal/modules/cjs/loader:1115:32)
    at Module._load (node:internal/modules/cjs/loader:962:12)
    at Module.require (node:internal/modules/cjs/loader:1139:19)

One problem is that the tests are green although such errors happen. But the origin of the bug is require('ep_etherpad-lite/node_modules/log4js');

In the future plugins cannot request modules like this, but should use require('log4js'). In this case it would work, but it would rely on core to not drop log4js. So best would be if plugins state the dependency in their own package.json

webzwo0i commented 11 months ago

Does anybody use Windows and can adapt bin/installOnWindows.bat? I failed badly... Most importantly is probably to check if NODE_ENV==production and change the npm link invocation accordingly.

webzwo0i commented 11 months ago

After the change, we will no longer have any problems with hoisting, as everything is in ./node_modules. npm's hoisting behavior also caused us some trouble in the past.

I checked which public plugins need to be adapted, see them below. Unfortunately this is a large amount, so I'll add a backwards compatible fix.

log4js

ep_auth_citizenos/package/serverHooks.js:const logger = require('ep_etherpad-lite/node_modules/log4js').getLogger(PLUGIN_NAME);
ep_button_link/package/index.js:const log4js = require('ep_etherpad-lite/node_modules/log4js');
ep_chat_log_join_leave/package/index.js:const log4js = require('ep_etherpad-lite/node_modules/log4js');
ep_comments_page/package/commentManager.js:const log4js = require('ep_etherpad-lite/node_modules/log4js');
ep_comments_page2/package/commentManager.js:const log4js = require('ep_etherpad-lite/node_modules/log4js');
ep_comments_page_short_edition/package/commentManager.js:const log4js = require('ep_etherpad-lite/node_modules/log4js');
ep_delete_after_delay/package/delete.js:const log4js = require('ep_etherpad-lite/node_modules/log4js');
ep_delete_empty_pads/package/index.js:const log4js = require('ep_etherpad-lite/node_modules/log4js');
ep_disable_imports/package/index.js:const log4js = require('ep_etherpad-lite/node_modules/log4js');
ep_headerauth/package/index.js:const log4js = require('ep_etherpad-lite/node_modules/log4js');
ep_mammoth/package/import.js:const log4js = require('ep_etherpad-lite/node_modules/log4js');
ep_sciencemesh/package/index.js:const log4js = require('ep_etherpad-lite/node_modules/log4js')
ep_webhooks/package/serverHooks.js:const logger = require('ep_etherpad-lite/node_modules/log4js').getLogger(PLUGIN_NAME);
ep_wrtc_heading/package/index.js:var log4js = require("ep_etherpad-lite/node_modules/log4js")

supertest

ep_font_color/package/static/tests/backend/specs/api/exportHTML.js:const supertest = require('ep_etherpad-lite/node_modules/supertest');
ep_font_color2/package/static/tests/backend/specs/api/exportHTML.js:const supertest = require('ep_etherpad-lite/node_modules/supertest');
ep_font_color3/package/static/tests/backend/specs/api/exportHTML.js:const supertest = require('ep_etherpad-lite/node_modules/supertest');
ep_ice_font_color/package/static/tests/backend/specs/api/exportHTML.js:const supertest = require('ep_etherpad-lite/node_modules/supertest');
ep_mediawiki/package/static/tests/backend/utils.js:const supertest = require('ep_etherpad-lite/node_modules/supertest');

express

ep_ether-o-meter/package/index.js:const express = require('ep_etherpad-lite/node_modules/express');
ep_fileupload/package/hooks.js:const express = require('ep_etherpad-lite/node_modules/express');
ep_fileupload_aws/package/hooks.js:  , express = require('ep_etherpad-lite/node_modules/express')
ep_imageconvert/package/hooks.js:const express = require('ep_etherpad-lite/node_modules/express');
ep_mypads/package/api.js:  express = require('ep_etherpad-lite/node_modules/express');

async-stacktrace

ep_full_hyperlinks/package/links.js:var ERR = require("ep_etherpad-lite/node_modules/async-stacktrace");
ep_full_hyperlinks/package/linkManager.js:var ERR = require("ep_etherpad-lite/node_modules/async-stacktrace");
ep_maadix/package/hooks.js:var ERR = require("ep_etherpad-lite/node_modules/async-stacktrace");

async

ep_custom_styles/package/index.js:       async = require('ep_etherpad-lite/node_modules/async'),
ep_delete_after_delay/package/delete.js:  async             = require('ep_etherpad-lite/node_modules/async'),
ep_message_all/package/index.js:const async = require('ep_etherpad-lite/node_modules/async');
ep_new_pad/package/index.js:var async = require('ep_etherpad-lite/node_modules/async');
ep_pads_stats/package/index.js:    async      = require('ep_etherpad-lite/node_modules/async');
ep_search/package/index.js:const async = require('ep_etherpad-lite/node_modules/async');
ep_who_did_what/package/exportWhoDidWhat.js:const async = require('ep_etherpad-lite/node_modules/async');

others

ep_comments_page_short_edition/package/exportHTML.js:const $ = require('ep_etherpad-lite/node_modules/cheerio');
ep_comments_page_short_edition/package/index.js:const formidable = require('ep_etherpad-lite/node_modules/formidable');

webzwo0i commented 11 months ago

The async-stacktrace dependency was already removed in ebe05f8e632f72c90ad3114e702ee3fe97d97ba9, so those plugins probably don't work anyway. When it comes to the other dependencies, I added symlinks in src/node_modules so the plugins should not break if we update and we can add PRs to those plugins.

JohnMcLear commented 11 months ago

Wow! This is amazing, great job!

We have software that can automatically resolve/remedy plugin issues provided by the foundation. https://github.com/ether/etherpad-lite/blob/develop/src/bin/plugins/checkPlugin.js -- I'd recommend using this. Would this break plugins on older versions of Etherpad tho? If so we should set the dependency within the plugin to depend on a newer version of Etherpad.

webzwo0i commented 11 months ago

Note to self: There are still some errors in admin tests, although node 16 was green multiple times in a row now. These are from node 18 and 20, seems flaky but need further investigation

 [chrome latest, Windows 10]     -> PASSED : Attempt to Update a plugin (minor version update)   6685 ms
[chrome latest, Windows 10] 
[2023-07-06 06:26:35.463] [INFO] http - HTTP server listening for connections
[chrome latest, Windows 10]     -> FAILED : "before each" hook for "Attempt to Install a plugin" Error: waitFor condition never became true ()=>helper.admin$&&helper.admin$(".menu").find("li").length>=3
[chrome latest, Windows 10]         at helper.waitFor (helper.js:209:9)
[chrome latest, Windows 10]         at helper.waitForPromise (helper.js:256:59)
[chrome latest, Windows 10]         at Context.<anonymous> (http://localhost:9001/javascripts/lib/ep_***-lite/tests/frontend/specs/adminupdateplugins.js?callback=testRunnerRequire.define:1:522)
[chrome latest, Windows 10] 
[chrome latest, Windows 10] FINISHED - but not all tests returned 11 tests passed, 1 tests failed, 0 tests pending, duration: 0:57
[chrome latest, Windows 10] 13 tests, but only 11 returned. Check for failed before/beforeEach-hooks (no `test end` is called for them and subsequent tests of the same suite are skipped), see https://github.com/mochajs/mocha/pull/1043

[chrome latest, Windows 10]     -> FAILED : Attempt to Uninstall a plugin Error: waitFor condition never became true ()=>0===helper.admin$("#installed-plugins .ep_headings2").length
[chrome latest, Windows 10]         at helper.waitFor (helper.js:209:9)
[chrome latest, Windows 10]         at helper.waitForPromise (helper.js:256:59)
[chrome latest, Windows 10]         at Context.<anonymous> (http://localhost:9001/javascripts/lib/ep_***-lite/tests/frontend/specs/adminupdateplugins.js?callback=testRunnerRequire.define:1:4076)
[chrome latest, Windows 10] 
[chrome latest, Windows 10] FINISHED - 12 tests passed, 1 tests failed, 0 pending, duration: 3:55
[chrome latest, Windows 10] 

 [chrome latest, Windows 10]     -> FAILED : Attempt to Uninstall a plugin Error: waitFor condition never became true ()=>0===helper.admin$("#installed-plugins .ep_headings2").length
[chrome latest, Windows 10]         at helper.waitFor (helper.js:209:9)
[chrome latest, Windows 10]         at helper.waitForPromise (helper.js:256:59)
[chrome latest, Windows 10]         at Context.<anonymous> (http://localhost:9001/javascripts/lib/ep_***-lite/tests/frontend/specs/adminupdateplugins.js?callback=testRunnerRequire.define:1:4076)
[chrome latest, Windows 10] 
[chrome latest, Windows 10] FINISHED - 12 tests passed, 1 tests failed, 0 pending, duration: 4:12

JohnMcLear commented 11 months ago

jose@ryzin:~/etherpad-lite$ bin/run.sh 
Copy the settings template to settings.json...
Removing node_modules and src/node_modules.
Linking src as new package ep_etherpad-lite
npm ERR! code EACCES
npm ERR! syscall symlink
npm ERR! path ../../../home/jose/etherpad-lite/src
npm ERR! dest /usr/lib/node_modules/ep_etherpad-lite
npm ERR! errno -13
npm ERR! Error: EACCES: permission denied, symlink '../../../home/jose/etherpad-lite/src' -> '/usr/lib/node_modules/ep_etherpad-lite'
npm ERR!  [Error: EACCES: permission denied, symlink '../../../home/jose/etherpad-lite/src' -> '/usr/lib/node_modules/ep_etherpad-lite'] {
npm ERR!   errno: -13,
npm ERR!   code: 'EACCES',
npm ERR!   syscall: 'symlink',
npm ERR!   path: '../../../home/jose/etherpad-lite/src',
npm ERR!   dest: '/usr/lib/node_modules/ep_etherpad-lite'
npm ERR! }
npm ERR! 
npm ERR! The operation was rejected by your operating system.
npm ERR! It is likely you do not have the permissions to access this file as the current user
npm ERR! 
npm ERR! If you believe this might be a permissions issue, please double-check the
npm ERR! permissions of the file and its containing directories, or try running
npm ERR! the command again as root/Administrator.

npm ERR! A complete log of this run can be found in: /home/jose/.npm/_logs/2023-07-06T13_29_09_788Z-debug-0.log
Installing dependencies...
Installing dev dependencies
npm ERR! code EACCES
npm ERR! syscall mkdir
npm ERR! path /usr/lib/node_modules/ep_etherpad-lite
npm ERR! errno -13
npm ERR! Error: EACCES: permission denied, mkdir '/usr/lib/node_modules/ep_etherpad-lite'
npm ERR!  [Error: EACCES: permission denied, mkdir '/usr/lib/node_modules/ep_etherpad-lite'] {
npm ERR!   errno: -13,
npm ERR!   code: 'EACCES',
npm ERR!   syscall: 'mkdir',
npm ERR!   path: '/usr/lib/node_modules/ep_etherpad-lite'
npm ERR! }
npm ERR! 
npm ERR! The operation was rejected by your operating system.
npm ERR! It is likely you do not have the permissions to access this file as the current user
npm ERR! 
npm ERR! If you believe this might be a permissions issue, please double-check the
npm ERR! permissions of the file and its containing directories, or try running
npm ERR! the command again as root/Administrator.

npm ERR! A complete log of this run can be found in: /home/jose/.npm/_logs/2023-07-06T13_29_10_084Z-debug-0.log

It works if I run w/ sudo.

By works I mean I can access the new pad page but if I try to create a new pad I get an error -_- Not sure how tests are passing!

SamTV12345 commented 11 months ago

jose@ryzin:~/etherpad-lite$ bin/run.sh 
Copy the settings template to settings.json...
Removing node_modules and src/node_modules.
Linking src as new package ep_etherpad-lite
npm ERR! code EACCES
npm ERR! syscall symlink
npm ERR! path ../../../home/jose/etherpad-lite/src
npm ERR! dest /usr/lib/node_modules/ep_etherpad-lite
npm ERR! errno -13
npm ERR! Error: EACCES: permission denied, symlink '../../../home/jose/etherpad-lite/src' -> '/usr/lib/node_modules/ep_etherpad-lite'
npm ERR!  [Error: EACCES: permission denied, symlink '../../../home/jose/etherpad-lite/src' -> '/usr/lib/node_modules/ep_etherpad-lite'] {
npm ERR!   errno: -13,
npm ERR!   code: 'EACCES',
npm ERR!   syscall: 'symlink',
npm ERR!   path: '../../../home/jose/etherpad-lite/src',
npm ERR!   dest: '/usr/lib/node_modules/ep_etherpad-lite'
npm ERR! }
npm ERR! 
npm ERR! The operation was rejected by your operating system.
npm ERR! It is likely you do not have the permissions to access this file as the current user
npm ERR! 
npm ERR! If you believe this might be a permissions issue, please double-check the
npm ERR! permissions of the file and its containing directories, or try running
npm ERR! the command again as root/Administrator.

npm ERR! A complete log of this run can be found in: /home/jose/.npm/_logs/2023-07-06T13_29_09_788Z-debug-0.log
Installing dependencies...
Installing dev dependencies
npm ERR! code EACCES
npm ERR! syscall mkdir
npm ERR! path /usr/lib/node_modules/ep_etherpad-lite
npm ERR! errno -13
npm ERR! Error: EACCES: permission denied, mkdir '/usr/lib/node_modules/ep_etherpad-lite'
npm ERR!  [Error: EACCES: permission denied, mkdir '/usr/lib/node_modules/ep_etherpad-lite'] {
npm ERR!   errno: -13,
npm ERR!   code: 'EACCES',
npm ERR!   syscall: 'mkdir',
npm ERR!   path: '/usr/lib/node_modules/ep_etherpad-lite'
npm ERR! }
npm ERR! 
npm ERR! The operation was rejected by your operating system.
npm ERR! It is likely you do not have the permissions to access this file as the current user
npm ERR! 
npm ERR! If you believe this might be a permissions issue, please double-check the
npm ERR! permissions of the file and its containing directories, or try running
npm ERR! the command again as root/Administrator.

npm ERR! A complete log of this run can be found in: /home/jose/.npm/_logs/2023-07-06T13_29_10_084Z-debug-0.log

It works if I run w/ sudo.

By works I mean I can access the new pad page but if I try to create a new pad I get an error -_- Not sure how tests are passing!

It works on my end. All tests are passing so far on Linux.

SamTV12345 commented 11 months ago

The tests fail because the complete return of the express server is undefined. This is only on Windows. I have no clue why.

webzwo0i commented 11 months ago

Thanks for testing. I see similar permission problems when building the Dockerfile. Will investigate.

webzwo0i commented 11 months ago

@JohnMcLear The permission problem is due to npm link using a default prefix where it installs the packages. In the Dockerfile it is easy to fix (npm config set prefix ${EP_DIR}) so a user-local directory is used.

It affects all other projects that use npm under the same user, but I think we could add the above line to installDeps.sh, or maybe something like: If npm config has no prefix set, log a message that the user should set one and exit installDeps.

webzwo0i commented 11 months ago

We have software that can automatically resolve/remedy plugin issues provided by the foundation. https://github.com/ether/etherpad-lite/blob/develop/src/bin/plugins/checkPlugin.js -- I'd recommend using this. Would this break plugins on older versions of Etherpad tho? If so we should set the dependency within the plugin to depend on a newer version of Etherpad.

OK, I can add a hint in checkPlugin.js. Plugins won't break, because I add symlinks for those dependencies that I could find (cheerio, express, formidable etc) that were in use by plugins. So require('ep_etherpad-lite/node_modules/express') will work. That's a bad hack, but as the fix (ie 's#ep_etherpad-lite/node_modules##g') is so trivial, it should be easy to update most of the plugins right after the PR would be released

webzwo0i commented 11 months ago

Regarding npm link I found another solution that does not require any change to local configs. npm link ep_etherpad-lite --bin-links=false won't try to install the binaries (etherpad-healthcheck etc) to the prefix directory. Testing now

The upgrade-from-last-release workflow also works now. It has some changes that need to be reverted after releasing.

JohnMcLear commented 11 months ago

cc @webzwo0i

jose@ryzin:~/etherpad-lite$ git checkout fix-admintests 
Branch 'fix-admintests' set up to track remote branch 'fix-admintests' from 'origin'.
Switched to a new branch 'fix-admintests'
jose@ryzin:~/etherpad-lite$ bin/run.sh 
Copy the settings template to settings.json...
Removing src/node_modules.
Linking src as new package ep_etherpad-lite.

up to date, audited 3 packages in 460ms

found 0 vulnerabilities
Installing dependencies...
Installing dev dependencies
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@azure/msal-node@1.18.0',
npm WARN EBADENGINE   required: { node: '10 || 12 || 14 || 16 || 18' },
npm WARN EBADENGINE   current: { node: 'v20.3.1', npm: '9.6.7' }
npm WARN EBADENGINE }
npm ERR! code EACCES
npm ERR! syscall mkdir
npm ERR! path /home/jose/node_modules/@gar/promisify
npm ERR! errno -13
npm ERR! Error: EACCES: permission denied, mkdir '/home/jose/node_modules/@gar/promisify'
npm ERR!  [Error: EACCES: permission denied, mkdir '/home/jose/node_modules/@gar/promisify'] {
npm ERR!   errno: -13,
npm ERR!   code: 'EACCES',
npm ERR!   syscall: 'mkdir',
npm ERR!   path: '/home/jose/node_modules/@gar/promisify'
npm ERR! }
npm ERR! 
npm ERR! The operation was rejected by your operating system.
npm ERR! It is likely you do not have the permissions to access this file as the current user
npm ERR! 
npm ERR! If you believe this might be a permissions issue, please double-check the
npm ERR! permissions of the file and its containing directories, or try running
npm ERR! the command again as root/Administrator.

npm ERR! A complete log of this run can be found in: /home/jose/.npm/_logs/2023-07-07T15_25_07_561Z-debug-0.log
jose@ryzin:~/etherpad-lite$ git pull
Already up-to-date.

JohnMcLear commented 11 months ago

I can nuke my /home/jose/node_modules and retry..

I nuked node_modules and retried and it works fine other than error when trying to get to a pad

An error occurred while loading the page:

Uncaught Error: The module at "security" does not exist.
in http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910
at line 1:4157

Stack trace:

Error: The module at "security" does not exist.
    at B (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:4163)
    at http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:4549
    at D (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:4556)
    at n (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:4909)
    at (module ep_etherpad-lite/static/js/security.js) (http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=c7839910:4:160)
    at k (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:2890)
    at l (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:3097)
    at i (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:3136)
    at q (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:3161)
    at L (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:3314)

webzwo0i commented 11 months ago

I can nuke my /home/jose/node_modules and retry..

I nuked node_modules and retried and it works fine other than error when trying to get to a pad

An error occurred while loading the page:

Uncaught Error: The module at "security" does not exist.
in http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910
at line 1:4157

Stack trace:

Error: The module at "security" does not exist.
    at B (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:4163)
    at http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:4549
    at D (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:4556)
    at n (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:4909)
    at (module ep_etherpad-lite/static/js/security.js) (http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=c7839910:4:160)
    at k (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:2890)
    at l (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:3097)
    at i (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:3136)
    at q (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:3161)
    at L (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:3314)

That should be fixed with https://github.com/ether/etherpad-lite/pull/5812/files#diff-51658006a7d10c89a5d654c15b55a916dd3c4e1a1bd2462632f0116884e5f47eR166

Can you try reloading with ctrl+shift+r?

JohnMcLear commented 11 months ago

I forgot I'd enabled cache so I went ahead and disabled it..

[2023-07-07 16:43:19.243] [INFO] plugins - npm --version: 9.6.7

I get this error still:

An error occurred while loading the page:

    Error: The module at "security" does not exist.

in http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6
at line 1:4163

Stack trace:

    B@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4163
    D/require<@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4549
    D@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4556
    n@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4909
    (module ep_etherpad-lite/static/js/security.js)@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=e2015cc6:4:160
    k@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:2890
    l@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3098
    i@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3136
    q@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3162
    L@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3315
    B@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4146
    D/require<@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4549
    D@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4556
    n@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4909
    (module ep_etherpad-lite/static/js/pad_utils.js)@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=e2015cc6:2:169
    k@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:2890
    l@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3098
    i@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3136
    q@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3162
    L@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3315
    B@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4146
    D/require<@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4549
    D@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4556
    n@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4909
    (module ep_etherpad-lite/static/js/pad.js)@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=e2015cc6:1:272
    k@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:2890
    l@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3098
    i@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3136
    q@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3162
    L@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3315
    B@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4146
    D/require<@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4549
    D@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4556
    n@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4909
    @http://127.0.0.1:9001/p/_-RHnxGgM75QFapuzAUq:729:28
    setTimeout handler*s@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:40:355
    (module ep_etherpad-lite/static/js/pluginfw/client_plugins.js)/exports.update/<@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:40:596
    u@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:23630
    fireWith@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:24394
    E@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:73488
    (module ep_etherpad-lite/static/js/vendors/jquery.js)/</</send/n/<@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:75786
    EventHandlerNonNull*send@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:75972
    ajax@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:71999
    (module ep_etherpad-lite/static/js/vendors/jquery.js)/</</v[t]@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:73943
    getJSON@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:73755
    (module ep_etherpad-lite/static/js/pluginfw/client_plugins.js)/exports.update@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:40:368
    @http://127.0.0.1:9001/p/_-RHnxGgM75QFapuzAUq:719:17
    @http://127.0.0.1:9001/p/_-RHnxGgM75QFapuzAUq:741:8

I set maxAge to 0 and minify to false but still get this error.

webzwo0i commented 11 months ago

I can nuke my /home/jose/node_modules and retry..

I nuked node_modules and retried and it works fine other than error when trying to get to a pad

An error occurred while loading the page:

Uncaught Error: The module at "security" does not exist.
in http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910
at line 1:4157

Stack trace:

Error: The module at "security" does not exist.
    at B (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:4163)
    at http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:4549
    at D (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:4556)
    at n (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:4909)
    at (module ep_etherpad-lite/static/js/security.js) (http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=c7839910:4:160)
    at k (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:2890)
    at l (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:3097)
    at i (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:3136)
    at q (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:3161)
    at L (http://127.0.0.1:9001/static/js/require-kernel.js?v=c7839910:1:3314)

That should be fixed with https://github.com/ether/etherpad-lite/pull/5812/files#diff-51658006a7d10c89a5d654c15b55a916dd3c4e1a1bd2462632f0116884e5f47eR166

Can you try reloading with ctrl+shift+r?

I forgot I'd enabled cache so I went ahead and disabled it..

[2023-07-07 16:43:19.243] [INFO] plugins - npm --version: 9.6.7

I get this error still:

An error occurred while loading the page:

    Error: The module at "security" does not exist.

in http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6
at line 1:4163

Stack trace:

    B@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4163
    D/require<@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4549
    D@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4556
    n@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4909
    (module ep_etherpad-lite/static/js/security.js)@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=e2015cc6:4:160
    k@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:2890
    l@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3098
    i@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3136
    q@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3162
    L@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3315
    B@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4146
    D/require<@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4549
    D@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4556
    n@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4909
    (module ep_etherpad-lite/static/js/pad_utils.js)@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=e2015cc6:2:169
    k@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:2890
    l@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3098
    i@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3136
    q@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3162
    L@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3315
    B@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4146
    D/require<@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4549
    D@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4556
    n@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4909
    (module ep_etherpad-lite/static/js/pad.js)@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=e2015cc6:1:272
    k@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:2890
    l@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3098
    i@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3136
    q@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3162
    L@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:3315
    B@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4146
    D/require<@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4549
    D@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4556
    n@http://127.0.0.1:9001/static/js/require-kernel.js?v=e2015cc6:1:4909
    @http://127.0.0.1:9001/p/_-RHnxGgM75QFapuzAUq:729:28
    setTimeout handler*s@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:40:355
    (module ep_etherpad-lite/static/js/pluginfw/client_plugins.js)/exports.update/<@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:40:596
    u@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:23630
    fireWith@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:24394
    E@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:73488
    (module ep_etherpad-lite/static/js/vendors/jquery.js)/</</send/n/<@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:75786
    EventHandlerNonNull*send@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:75972
    ajax@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:71999
    (module ep_etherpad-lite/static/js/vendors/jquery.js)/</</v[t]@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:73943
    getJSON@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:32:73755
    (module ep_etherpad-lite/static/js/pluginfw/client_plugins.js)/exports.update@http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/ace2_common.js?callback=require.define&v=e2015cc6:40:368
    @http://127.0.0.1:9001/p/_-RHnxGgM75QFapuzAUq:719:17
    @http://127.0.0.1:9001/p/_-RHnxGgM75QFapuzAUq:741:8

I set maxAge to 0 and minify to false but still get this error.

The stack trace above indicates minify is still true?! Can you try with open developer console->network tab->"disable cache"? Although this is still a bad sign even if it would work, because most clients won't be so tech savvy and try all of this... :-(

SamTV12345 commented 11 months ago

Good job so far. The startup is now insane fast because it doesn't start to redownload anything. I noticed there is a package.json file in the root of the project when running ./src/bin/run.sh maybe that should be included in the gitignore.

JohnMcLear commented 11 months ago

@webzwo0i I'm 100% minify is set to false and I have "Disable Cache" checked.

I'm defo hitting the right instance too as if I kill it I get failure to connect.

SamTV12345 commented 11 months ago

Is it really correct to remove all package.json?

Deleted file - D:\a\etherpad-lite\etherpad-lite\src\node_modules\async
Deleted file - D:\a\etherpad-lite\etherpad-lite\src\node_modules\cheerio
Deleted file - D:\a\etherpad-lite\etherpad-lite\src\node_modules\express
Deleted file - D:\a\etherpad-lite\etherpad-lite\src\node_modules\formidable
Deleted file - D:\a\etherpad-lite\etherpad-lite\src\node_modules\log4js
Deleted file - D:\a\etherpad-lite\etherpad-lite\src\node_modules\supertest
Deleting old package.json and package-lock.json
Deleted file - D:\a\etherpad-lite\etherpad-lite\src\package.json
Deleted file - D:\a\etherpad-lite\etherpad-lite\src\bin\doc\package.json
Deleted file - D:\a\etherpad-lite\etherpad-lite\src\package-lock.json
Deleted file - D:\a\etherpad-lite\etherpad-lite\src\bin\doc\package-lock.json

SamTV12345 commented 11 months ago

@webzwo0i I'm 100% minify is set to false and I have "Disable Cache" checked.

I'm defo hitting the right instance too as if I kill it I get failure to connect.

Weird. I don't get that problem on my Mac. Did you try reinstalling the project? I'm using v20.3.1 with 9.8.0. I have set minify to true, maxAge to 21600. It also works with minify: false and maxAge: 0

SamTV12345 commented 11 months ago

I don't know why but ep_etherpad_lite version 1.8.14 is in the package-lock.json bringing in npm version 6.14.13 but I can't find where this version 1.8.14 is coming from.

SamTV12345 commented 11 months ago

Found it: https://www.npmjs.com/package/ep_etherpad-lite?activeTab=readme How do we update that?

SamTV12345 commented 11 months ago

Nice only Windows failing. I'm wondering why it fails there. It works on my Windows machine up until the point the httpserver address is retrieved.

webzwo0i commented 11 months ago

The admin tests still seem to fail from time to time, will fix this. (Probably a problem with package.json in root directory, that now contains an entry for ep_etherpad-lite, which is not a real plugin.)

The windows problem seems to be "fixed". The problem is that binaries like mocha, which are stored in ./node_modules/bin are not available right away, only when running "npm i" again (after ./src/bin/installDeps.sh). We should find a real solution... It seems to work on linux without calling npm i, so I assume we could add another npm task (copy over test to test-on-windows in package.json), but my first attempts did not succeed. Maybe we can add a manual symlink for mocha

webzwo0i commented 11 months ago

This problem btw also exists on linux, so I adapted package.json to not invoke "mocha" but use the path "../node_modules/mocha/bin/_mocha". This needs to be researched, how to make npm link install the binaries. (--bin-links seems to be only for etherpad, etherpad-healthcheck etc. not the binaries in ./node_modules/.bin but I might be wrong)

webzwo0i commented 11 months ago

OK so it turns out that --bin-links=false is a bad decision. We should find a solution for npm config set prefix that works on windows and linux, maybe even set it to ./node_modules/.bin so that npm can find mocha etc.

webzwo0i commented 11 months ago

It's nearly finished, if we find the reason for John's problem from above. :-)

Current problems:

non-idiomatic use of scripts in src/package.json, ie "mocha" is run as "node_modules/mocha/bin/...". This is due to the problem with --bin-links and permissions, in case someone runs a not-configured npm environment (like we do in Docker or during Windows builds). Otherwise it would try to install packages to the global node_modules directory, which is probably not writable to the user.
admin/plugins uninstalation (see below)

todo:

enhance checkPlugin script to fix compatibility problem when requiring packages that are only in core, but not in the plugin.

I squashed and reordered commits.

The problem with frontend admin tests are 50% solved, e.g. I added a version string to install/update. In past versions this wasn't needed, as there was no package.json, so npm always installed the latest version. The other half is uninstallation. My current guess is, that it's having problems during restart, not exactly npm-related.

I'm off for some days now, but can finish it next week.

Two other problems, that are not related to the PR:

There is a strange problem with frontend tests on Edge, e.g. https://app.saucelabs.com/tests/ee1bb2fff782431dae62c84082fde852#85 It's erroring right after loading the tests, but in the video the error is not readable and there seems to be no console.log here. It works in most cases, though.

in remote_runner.js. Sometimes, even when jobs fail, it won't exit with a non-zero exit code, e.g. https://github.com/ether/etherpad-lite/actions/runs/5494870057/jobs/10013912362?pr=5812#step:15:256 This is not related to this PR. If some tests are non-intentionally skipped, the return value must be non-zero

[chrome latest, Windows 10] FINISHED - but not all tests returned 11 tests passed, 1 tests failed, 0 tests pending, duration: 0:58
[chrome latest, Windows 10] 13 tests, but only 11 returned. Check for failed before/beforeEach-hooks (no `test end` is called for them and subsequent tests of the same suite are skipped), see https://github.com/mochajs/mocha/pull/1043

@JohnMcLear Can you try again and if it does not work, print the output of npm config get inside and outside your etherpad directory. Also post the result of find $etherpad-directory -name security -type d so we know that the dependency installation worked. Do you test on linux or windows?

SamTV12345 commented 11 months ago

@webzwo0i Don't know if this is related to your pr. There is still the two year old etherpad lite version in my dev dependencies. Can we get rid of that?

webzwo0i commented 11 months ago

I don't have credentials to push to npmjs.com but yeah we should update the version there.

iirc this is pulled in via etherpad-cli-client in devDependencies.

Or do you mean ep_etherpad-lite is stated in your package.json's devDependencies? If yes, try to checkout the package.json from this branch as there shouldn't be one on this branch and try to reproduce when the devDependency is added. Afaik npm ci or npm i or npm link shouldn't do that.

SamTV12345 commented 11 months ago

I don't have credentials to push to npmjs.com but yeah we should update the version there.

iirc this is pulled in via etherpad-cli-client in devDependencies.

Maybe another github action for release would be good. I think the npm token is in the organization so the runner should be able to read and update the package on every release.

SamTV12345 commented 11 months ago

I can't link to it as the package-lock.json is too large. Search for "1.8.14"

JohnMcLear commented 11 months ago

jose@ryzin:~$ rm -Rf etherpad-lite
jose@ryzin:~$ git clone git@github.com:ether/etherpad-lite.git
Cloning into 'etherpad-lite'...
...
jose@ryzin:~/etherpad-lite$ git checkout fix-admintests 
Switched to a new branch 'fix-admintests'
jose@ryzin:~/etherpad-lite$ git pull
Already up-to-date.
jose@ryzin:~/etherpad-lite$ bin/run.sh 
...
[2023-07-09 10:53:32.527] [INFO] server - Etherpad is running
[2023-07-09 10:53:38.962] [INFO] Minify - Compress JS file js/vendors/html10n.js.
...

Results in browser error

An error occurred while loading the page:

Uncaught Error: The module at "security" does not exist.
in http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5
at line 1:4157

Stack trace:

Error: The module at "security" does not exist.
    at B (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:4163)
    at http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:4549
    at D (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:4556)
    at n (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:4909)
    at (module ep_etherpad-lite/static/js/security.js) (http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=8c84b2c5:4:160)
    at k (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:2890)
    at l (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:3097)
    at i (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:3136)
    at q (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:3161)
    at L (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:3314)

jose@ryzin:~/etherpad-lite$ npm config get
; "user" config from /home/jose/.npmrc

//registry.npmjs.org/:_authToken = (protected) 
fund = "https://etherpad.org/donate" 

; node bin location = /usr/bin/node
; node version = v20.3.1
; npm local prefix = /home/jose
; npm version = 9.6.7
; cwd = /home/jose/etherpad-lite
; HOME = /home/jose
; Run `npm config ls -l` to show all defaults.

jose@ryzin:~$ npm config get
; "user" config from /home/jose/.npmrc

//registry.npmjs.org/:_authToken = (protected) 
fund = "https://etherpad.org/donate" 

; node bin location = /usr/bin/node
; node version = v20.3.1
; npm local prefix = /home/jose
; npm version = 9.6.7
; cwd = /home/jose
; HOME = /home/jose
; Run `npm config ls -l` to show all defaults.

jose@ryzin:~/etherpad-lite$ find /home/jose/etherpad-lite -name security -type d 
/home/jose/etherpad-lite/src/node/security

Ubuntu

SamTV12345 commented 11 months ago

jose@ryzin:~$ rm -Rf etherpad-lite
jose@ryzin:~$ git clone git@github.com:ether/etherpad-lite.git
Cloning into 'etherpad-lite'...
...
jose@ryzin:~/etherpad-lite$ git checkout fix-admintests 
Switched to a new branch 'fix-admintests'
jose@ryzin:~/etherpad-lite$ git pull
Already up-to-date.
jose@ryzin:~/etherpad-lite$ bin/run.sh 
...
[2023-07-09 10:53:32.527] [INFO] server - Etherpad is running
[2023-07-09 10:53:38.962] [INFO] Minify - Compress JS file js/vendors/html10n.js.
...

Results in browser error

An error occurred while loading the page:

Uncaught Error: The module at "security" does not exist.
in http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5
at line 1:4157

Stack trace:

Error: The module at "security" does not exist.
    at B (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:4163)
    at http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:4549
    at D (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:4556)
    at n (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:4909)
    at (module ep_etherpad-lite/static/js/security.js) (http://127.0.0.1:9001/javascripts/lib/ep_etherpad-lite/static/js/pad.js?callback=require.define&v=8c84b2c5:4:160)
    at k (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:2890)
    at l (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:3097)
    at i (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:3136)
    at q (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:3161)
    at L (http://127.0.0.1:9001/static/js/require-kernel.js?v=8c84b2c5:1:3314)

jose@ryzin:~/etherpad-lite$ npm config get
; "user" config from /home/jose/.npmrc

//registry.npmjs.org/:_authToken = (protected) 
fund = "https://etherpad.org/donate" 

; node bin location = /usr/bin/node
; node version = v20.3.1
; npm local prefix = /home/jose
; npm version = 9.6.7
; cwd = /home/jose/etherpad-lite
; HOME = /home/jose
; Run `npm config ls -l` to show all defaults.

jose@ryzin:~$ npm config get
; "user" config from /home/jose/.npmrc

//registry.npmjs.org/:_authToken = (protected) 
fund = "https://etherpad.org/donate" 

; node bin location = /usr/bin/node
; node version = v20.3.1
; npm local prefix = /home/jose
; npm version = 9.6.7
; cwd = /home/jose
; HOME = /home/jose
; Run `npm config ls -l` to show all defaults.

jose@ryzin:~/etherpad-lite$ find /home/jose/etherpad-lite -name security -type d 
/home/jose/etherpad-lite/src/node/security

Ubuntu

I did follow these steps and it is working for me. Is the error maybe that you execute bin/run.sh instead of ./src/bin/run.sh. Otherwise I'll spin up a virtual machine with Ubuntu and try to reproduce this behavior. Are you on Jammy?

SamTV12345 commented 11 months ago

bin/run.sh also works even though it seems a little bit slower.

JohnMcLear commented 11 months ago

bin/run.sh also works even though it seems a little bit slower.

src/bin/run.sh doesn't work for me -- same error -_-

SamTV12345 commented 11 months ago

@JohnMcLear @webzwo0i When can we merge this into develop? I guess it would solve a lot of issues with people trying to install anything with a more current version.

I setup a blank VM, installed node and npm in the latest version and could succesfully start etherpad. So I guess there is something wrong with your config. Maybe the old npm is still somewhere present and messes with the packages.

webzwo0i commented 11 months ago

I saw that on some installations, npm link results in permission errors, because it tries to use a global path unless a more specific directory is given. Not sure what is the optimal directory, though. $HOME/.npm-packages? $etherpad-directory/.npm-packages?

And regarding "somewhere present" it would be good if we find the location, that messes things up. iirc John tried to remove every npm/node trace in his home directory. So I assume something like "sudo find /usr $HOME -name npm" should be done to find locations where npm is installed... We probably get some bug reports, but because we have no "clean install" script (nixos config etc.) and not everybody wants to use Docker, we cannot give good advice how to solve the problem other than "Remove any global node packages and try to start with a newly created user"

One solution could be to strip anything from NODE_PATH and use it in server.js, so that it is used no matter how Etherpad is started. See https://gist.github.com/branneman/8048520#7-the-hack Not sure if this is "stable" or if there is a better way that will still work in the future. This way we could remove any lookups outside Etherpad's directory, which is a good idea anyway, right? $NODE_PATH should be something like ${ETHERPAD_PATH}/src/node_modules:${ETHERPAD_PATH}/node_modules.

SamTV12345 commented 11 months ago

I saw that on some installations, npm link results in permission errors, because it tries to use a global path unless a more specific directory is given. Not sure what is the optimal directory, though. $HOME/.npm-packages? $etherpad-directory/.npm-packages?

And regarding "somewhere present" it would be good if we find the location, that messes things up. iirc John tried to remove every npm/node trace in his home directory. So I assume something like "sudo find /usr $HOME -name npm" should be done to find locations where npm is installed... We probably get some bug reports, but because we have no "clean install" script (nixos config etc.) and not everybody wants to use Docker, we cannot give good advice how to solve the problem other than "Remove any global node packages and try to start with a newly created user"

One solution could be to strip anything from NODE_PATH and use it in server.js, so that it is used no matter how Etherpad is started. See https://gist.github.com/branneman/8048520#7-the-hack Not sure if this is "stable" or if there is a better way that will still work in the future. This way we could remove any lookups outside Etherpad's directory, which is a good idea anyway, right? $NODE_PATH should be something like ${ETHERPAD_PATH}/src/node_modules:${ETHERPAD_PATH}/node_modules.

Thanks for the update. True. Forgot that there may be users that use it on bare metal instead of Docker. I agree that we should wait for that. $HOME/.npm-packages sounds good.

I'll try to reproduce the issue. Maybe with multiple npm installations it (does not) works.

JohnMcLear commented 11 months ago

@SamTV12345 I agree this should be merged but I am confident if we merge and release this we'll have a lot of bug reports reporting the same finding as me and we'll be forced to do a patch release to address it.

So while I'm also keen to see this, let's ensure we're covering my weird config/edge case first please :)

JohnMcLear commented 11 months ago

@webzwo0i this looks wrong https://github.com/ether/etherpad-lite/blob/fix-admintests/src/package-lock.json#L3103

JohnMcLear commented 11 months ago

https://github.com/ether/etherpad-lite/actions/runs/5597289742/jobs/10235522785?pr=5839#step:15:158

[2023-07-19 09:21:33.165] [INFO] plugins - Installing plugin ep_headings2@0.2.44...
Error: 023-07-19 09:21:35.154] [ERROR] runCmd|npm - npm WARN EBADENGINE Unsupported engine {
Error: 023-07-19 09:21:35.155] [ERROR] runCmd|npm - npm WARN EBADENGINE   package: 'ep_***-lite@1.9.1',
Error: 023-07-19 09:21:35.155] [ERROR] runCmd|npm - npm WARN EBADENGINE   required: { node: '>=16.20.1', npm: '>=9.8.0' },
Error: 023-07-19 09:21:35.155] [ERROR] runCmd|npm - npm WARN EBADENGINE   current: { node: 'v16.20.1', npm: '8.19.4' }
Error: 023-07-19 09:21:35.155] [ERROR] runCmd|npm - npm WARN EBADENGINE }

SamTV12345 commented 11 months ago

npm 9.8.0 is a bit too high as a requirement for node 16.