Skip to content

fix: declare missing param in native console.error shim - #67

Open
moodyjmz wants to merge 1 commit into
mainfrom
fix/native-console-error-missing-param
Open

fix: declare missing param in native console.error shim#67
moodyjmz wants to merge 1 commit into
mainfrom
fix/native-console-error-missing-param

Conversation

@moodyjmz

Copy link
Copy Markdown
Member

Problem

common/Native/native.js's console shim declares error with no parameter but its body references param:

error: function() { window.native && window.native.ConsoleLog("[error]: " + param); }

param exists only as a formal parameter of the sibling log/time/timeEnd functions, so every console.error() call in the DoctRenderer JS context throws ReferenceError: param is not defined. (Reading an undeclared identifier throws regardless of strict mode; only assignment differs.)

Why it matters

native.js is the first <file> entry in DocumentServer:build/configs/core/DoctRenderer.config, parsed into m_arrFiles by core:DesktopEditor/doctrenderer/config.h:117-140 and concatenated ahead of sdk-all.js by core:DesktopEditor/doctrenderer/editors.cpp:69-75. It is therefore live in every converter operation — word, cell, slide, visio, pdf.

The defect was dormant until sendClientLog() began routing error-level messages through console.error(). After that, any converter operation that logged an error threw instead of logging: DoctRenderer returns <result><error code="save" /></result> and exits 86. On the forced-save path that is silent data loss — the client reports "All changes saved" while nothing persists — and the original error is destroyed by the throw, so the logs say nothing useful about the actual fault.

Because it is a deterministic code bug rather than a race, the same docId fails on every retry.

The stack in the linked report matches this line byte-exactly: the deployed common/Native/native.js is 23 lines, and param sits at column 318 of line 23 — <anonymous>:23:318.

Blast radius

Not limited to spreadsheet co-editing, which is merely where it was first observed. DoctRenderer-reachable console.error( call sites include:

  • cell/apiBuilder.js:29161, word/apiBuilder.js:31653 — Document Builder API
  • common/macros.js:102,123,502 — macro execution
  • common/apiBase.js:384
  • pdf/build/api.js:188

sendClientLog("error", …) callers likewise span word/Editor/History.js:1024, slide/api.js, common/apiBase.js:362,1722 and cell/.

The fix

Declare the parameter. Deliberately the minimal change, matching the obvious upstream fix character-for-character so the eventual ONLYOFFICE sync merges cleanly — common/Native/native.js has seen 121 commits ever versus 1207 for common/editorscommon.js, and this is not a file worth diverging in.

Alternatives considered and rejected:

  • try/catch around the console.error(msg) call in sendClientLog — fixes one of ~17 call sites, leaves the defect in place, and puts the divergence in a heavily-churned file. Maximum future conflict, minimum coverage.
  • Variadic/defensive shim — a few callers do pass two arguments (common/clipboard_base.js:1124, common/serviceworker/document_editor_service_worker.js:184,305), but they silently drop the second rather than throwing. Worth a separate follow-up, not worth bundling here.

Testing

Nothing in tests/runAll.js or check-build.yml exercises the DoctRenderer bundle — those run browser QUnit via node-qunit-puppeteer, where console.error is the real platform function and this defect is unreachable by construction. That asymmetry is precisely why the bug shipped.

Building a native-path harness is not proportionate to a one-word fix. The proportionate guard is no-undef, which .eslintrc.yaml does not currently enable and which nothing wires into the build; running eslint with only that rule over common/Native/*.js flags this line and nothing else of substance. Follow-up PR to enable it and add it to the code-style gate is coming separately.

Verification for this PR: workflow_dispatch on DocumentServer's build.yml with sdkjs_branch set to this branch, then confirm line 23 of the deployed native.js in the built image.

Related

Reported downstream as Euro-Office/DocumentServer#307 (deliberately a cross-reference, not a closing keyword — GitHub closing keywords do not act across repositories, and that issue should stay open until an image actually ships this).

Upstream: ONLYOFFICE/DocumentServer#3711confirmed-bug + fixed-release, but still open, and no public ONLYOFFICE branch has any commit after 2026-05-26, so there is no upstream fix available to pull. Maintainer estimate for release is "beginning of autumn".

The native console shim's error() referenced `param` without declaring it as
a parameter, so every console.error() call inside the DoctRenderer JS context
threw `ReferenceError: param is not defined`.

Dormant until sendClientLog() started routing error-level messages through
console.error(), after which any converter operation that logged an error
aborted instead: DoctRenderer returned <error code="save"/> and exited 86.
On the forced-save path that meant silent data loss, with the original error
destroyed by the throw.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
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