Express v4.x does not check to see if a Promise returned from a
middleware function will be rejected, so explicitly pass the Promise
rejection reason to `next()`.
We can revert this change after we upgrade to Express v5.0.
See https://expressjs.com/en/guide/error-handling.html for details.
Before, an unhandled rejection or uncaught exception during startup
would cause `exports.exit()` to wait forever for startup completion.
Similarly, an error during shutdown would cause `exports.exit()` to
wait forever for shutdown to complete. Now any error during startup or
shutdown triggers an immediate exit.
Logging verbosity of the openapi handlers was turned down so GitHub
should be happier with INFO now. This makes it easier to troubleshoot
problems.
This reverts commit b98aaf4904.
`waitForPromise()` should always be used with `await` (either directly
or with a later `await` on the returned Promise). In this case,
the condition should be immediately true so `waitForPromise()` is not
the right tool here.
* Delete the unused `optDoNow` parameter from `pendingInit()`.
* Move the `setAuthorInfo()` 1st parameter check out of the wrapper
and in to the `setAuthorInfo()` function itself.
All of the tests in this file are commented out so this file does
nothing. We can uncomment the code and clean it up, but the approach
taken in these tests will never work: For security reasons, browsers
do not allow synthetic key events to perform the default
behavior (such as moving the carent when an arrow key is pressed).
There are two ways to test responses to navigation keys:
* Use WebDriver to create "genuine" keyboard events.
* Suppress the default behavior and implement caret movement
ourselves. This is tremendously complicated, especially arrow
up/down.
Also add symlinks from the old `bin/` and `tests/` locations to avoid
breaking scripts and other tools.
Motivations:
* Scripts and tests no longer have to do dubious things like:
require('ep_etherpad-lite/node_modules/foo')
to access packages installed as dependencies in
`src/package.json`.
* Plugins can access the backend test helper library in a non-hacky
way:
require('ep_etherpad-lite/tests/backend/common')
* We can delete the top-level `package.json` without breaking our
ability to lint the files in `bin/` and `tests/`.
Deleting the top-level `package.json` has downsides: It will cause
`npm` to print warnings whenever plugins are installed, npm will
no longer be able to enforce a plugin's peer dependency on
ep_etherpad-lite, and npm will keep deleting the
`node_modules/ep_etherpad-lite` symlink that points to `../src`.
But there are significant upsides to deleting the top-level
`package.json`: It will drastically speed up plugin installation
because `npm` doesn't have to recursively walk the dependencies in
`src/package.json`. Also, deleting the top-level `package.json`
avoids npm's horrible dependency hoisting behavior (where it moves
stuff from `src/node_modules/` to the top-level `node_modules/`
directory). Dependency hoisting causes numerous mysterious
problems such as silent failures in `npm outdated` and `npm
update`. Dependency hoisting also breaks plugins that do:
require('ep_etherpad-lite/node_modules/foo')
For some reason strings are sometimes passed to `findUnmet()`, which
is obviously unexpected given the way the code is written. Rather than
figure out why strings are passed and how to safely avoid passing
strings, just return early. The net effect is the same, but returning
early avoids setting a property on a string, which is prohibited in
strict mode.
Benefits of `callHookFnSync()` and `callHookFnAsync()`:
* They are a lot more forgiving than `hookCallWrapper()` was.
* They perform useful sanity checks.
* They have extensive unit test coverage.
* They make the behavior of `callFirst()` and `aCallFirst()` match
the behavior of `callAll()` and `aCallAll()`.
Define states and use them to properly handle multiple calls to
`start()`, `stop()`, and `exit()`. (Multiple calls to `exit()` can
happen if there is an uncaught exception or signal during shutdown.)
This should also make it easier to add support for cleanly restarting
the server after a shutdown (for tests or via an `/admin` page).
* lint: skin-variants
* for squash: Fix attachment of event listener
Before this PR the statement was outside the function. I'm assuming
the move into the function body was accidental, so move it back out.
* for squash: Preserve order of function calls
Co-authored-by: Richard Hansen <rhansen@rhansen.org>
There are some problems with nyc:
* The coverage numbers aren't useful in our case because most of the
code is executed outside the test process (the test code is mostly
API client logic).
* nyc messes with line numbers, which makes it much harder to debug
problems.
* We're seeing frequent SIGABRT crashes while nyc is printing the
results table. I'm not sure if nyc is the cause of the crashes, or
if it's making a race condition worse, or if the crashes have
nothing to do with nyc, but we don't lose much by removing it so
we might as well see if the crash frequency improves.
Before this change, the `author` attribute was silently discarded
during `.map()` iteration and the name of the attribute to remove was
included twice with two different values.
Before this commit, the callback passed to `.map()` during attribute
removal was a normal function, not an arrow function. This meant that
the value of `this` in the function body depended on how the callback
was invoked. In this case, the callback was invoked without any
explicit context (it was not called as a method, nor was it called via
`.call()`, `.apply()`, or `.bind()`). Without any explicit context,
the value of `this` depends on strict mode. Currently the function is
in sloppy mode, so `this` refers to the "global this" object (a.k.a.,
`window`). It doesn't make sense for the callback to reference
`window.author`, so I'm assuming the previous behavior was a bug.
Now the function is an arrow function, so the value of `this` comes
from the enclosing lexical context, which in this case is the
AttributeManager object. I believe that was the original intention.
