Skip to content

fix: lazy-require wordnet so the client bundle stops hanging the editor#92

Merged
JohnMcLear merged 2 commits into
mainfrom
fix/wordnet-lazy-require-editor-hang
Jun 22, 2026
Merged

fix: lazy-require wordnet so the client bundle stops hanging the editor#92
JohnMcLear merged 2 commits into
mainfrom
fix/wordnet-lazy-require-editor-hang

Conversation

@JohnMcLear

Copy link
Copy Markdown
Member

Symptom

ep_define's frontend CI (Node.js Packagefrontend / test-frontend) has timed out at the 6-hour job limit and been cancelled on every run for months — including unrelated Dependabot PRs (#89). With ep_define installed, the pad editor never loads in CI: the ace_outer iframe is never created, no JS error is thrown, and every spec — including core specs like a11y_dialogs — times out in beforeEach waiting for the editor.

It does not reproduce locally (the editor loads fine), which is why it went unfixed: the failure only manifests in the CI build of the client bundle.

Root cause

handleMessage.js required wordnet at module top level:

const wordnet = require('wordnet');

Etherpad's client bundler pulls plugin hook modules into the browser bundle. Evaluating wordnet's server-only module body (it reads the WordNet DB off disk) in the browser silently hangs the editor bootstrap before ace_outer is created — no exception, just a stuck "Loading…".

Bisected directly in CI with an instrumented pad-load probe:

Variant Editor
empty hooks ✅ loads
client hooks only ✅ loads
handleMessage hook (top-level require('wordnet')) ❌ hangs
no-op handleMessage (no require) ✅ loads
full plugin + lazy require (this PR) ✅ loads

Fix

  • Require wordnet lazily inside ensureInit(), so its module body only ever runs server-side, on an actual define request — never during client bundle evaluation.
  • Drive-by: stop returning [null] for non-define messages. That told core to drop every message (typing, cursor moves, …); it now returns nothing so only the plugin's own define messages are intercepted.

Companion to #90 (the pnpm-pin CI fix), which was necessary but not sufficient — this is the actual editor-load blocker.

🤖 Generated with Claude Code

ep_define's frontend CI has timed out (6h, cancelled) on every run since
the plugin started actually being installed in tests (Apr 2026), and the
pad editor never loaded in CI with ep_define present — `ace_outer` was
never created, no JS error was thrown, every spec (including core specs)
timed out waiting for the editor.

Root cause: `handleMessage.js` did `const wordnet = require('wordnet')`
at module top level. Etherpad's client bundler pulls plugin hook modules
into the browser bundle; evaluating wordnet's (server-only) module body
in the browser silently hangs the editor bootstrap. Bisected in CI: the
hook with the top-level require hangs the editor; a no-op handler (no
require) loads fine; moving the require lazily inside the hook with the
full plugin restored loads fine.

- Require `wordnet` lazily inside `ensureInit()` so its body only runs
  server-side, on an actual define request.
- Stop returning `[null]` for non-define messages: that told core to
  DROP every message (typing, cursor moves, …). Return nothing instead
  so only the plugin's own define messages are intercepted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Fix client bundle hang by lazy-loading wordnet in handleMessage hook
🐞 Bug fix 🕐 10-20 Minutes

Grey Divider

Description

• Lazy-require wordnet inside ensureInit() to avoid browser-bundle evaluation hangs.
• Preserve normal Etherpad message flow by not returning [null] for non-define messages.
Diagram

graph TD
  bundler["Etherpad bundler"] --> bundle["Client JS bundle"] --> hook["handleMessage.js hook"] --> lazy["ensureInit() lazy require"] --> wn["wordnet (server-only)"] --> db[(WordNet DB)]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Split server-only logic into a separate, non-bundled module
  • ➕ Keeps browser bundle free of any server-only dependency references
  • ➕ Makes the server/client boundary explicit and easier to audit
  • ➖ May require Etherpad/plugin build knowledge to ensure the server module is never pulled into the client bundle
  • ➖ Larger refactor than needed for the immediate CI/editor unblock
2. Bundler exclude/alias configuration for `wordnet` in client builds
  • ➕ Centralized guarantee that server-only deps won’t be evaluated in the browser
  • ➖ Depends on Etherpad’s bundling pipeline and may not be controllable from a plugin
  • ➖ Risk of regressions if bundler behavior changes across Etherpad versions

Recommendation: The PR’s lazy-require inside ensureInit() is the most pragmatic fix: it prevents client-side evaluation of wordnet without changing plugin structure or requiring bundler customization. Consider a follow-up refactor to separate server-only code paths if additional server-only dependencies are added later.

Files changed (1) +13 / -3

Bug fix (1) +13 / -3
handleMessage.jsLazy-load wordnet and fix hook return behavior for non-define messages +13/-3

Lazy-load wordnet and fix hook return behavior for non-define messages

• Moves 'require('wordnet')' behind 'ensureInit()' so the server-only module body is not executed during client bundle evaluation (preventing editor bootstrap hangs in CI). Adjusts 'handleMessage' to return nothing for non-define messages so Etherpad core continues processing normal collaboration traffic.

handleMessage.js

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 21, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Rejected initPromise never resets 🐞 Bug ☼ Reliability
Description
ensureInit() memoizes initPromise without clearing it on rejection, so a single wordnet.init()
failure causes every later define request to immediately fail. Because handleMessage() catches all
errors and responds with null definitions, initialization failures become silent and hard to
diagnose (the client just sees “No Match”).
Code

handleMessage.js[R10-15]

+let wordnet = null;
let initPromise = null;
const ensureInit = () => {
+  if (!wordnet) wordnet = require('wordnet');
if (!initPromise) initPromise = wordnet.init();
return initPromise;
Evidence
initPromise is assigned once and never reset, so a rejected wordnet.init() will be reused on
every call to ensureInit(). handleMessage() wraps ensureInit() and lookup() in a single
try/catch that converts any failure into sendDefinition(context, null), providing no server-side
signal that init failed.

handleMessage.js[10-50]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ensureInit()` stores the Promise returned by `wordnet.init()` in a module-global `initPromise`. If that Promise rejects once, it stays rejected forever and all subsequent define requests will fail immediately. Additionally, `handleMessage()` currently catches *all* errors and sends a `null` definition response, which hides initialization/configuration failures.
## Issue Context
This PR moved `require('wordnet')`/`wordnet.init()` into the request path (lazy init). That makes it more important to:
- avoid permanently poisoning future requests after one transient init failure
- surface init failures (log them) so operators can diagnose missing DB, permission issues, etc.
## Fix Focus Areas
- handleMessage.js[10-50]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

context.client.json.send() was the socket.io v2 API and no longer exists,
so the define reply never reached the client and the gritter notification
never appeared (define.spec.ts:13). Emit over context.socket and use the
server-resolved sessionInfo author/pad instead of client-supplied ids.
@JohnMcLear JohnMcLear merged commit e0d81bd into main Jun 22, 2026
3 checks passed
@JohnMcLear JohnMcLear deleted the fix/wordnet-lazy-require-editor-hang branch June 22, 2026 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant