Streaming WorkbookReader intermittently throws "Cannot read properties of undefined (reading 'sheets')" on Node ≥18
- Dominant language
- JavaScript
- Stars
- 15.5k
- Forks
- 2k
- PR merge metrics
- No merged PRs in 30d
Description
### Summary
`stream.xlsx.WorkbookReader` intermittently throws
```
TypeError: Cannot read properties of undefined (reading 'sheets')
at WorkbookReader._parseWorksheet (lib/stream/xlsx/workbook-reader.js:303)
```
on Node ≥ 18. It is a race, not a fixed-order bug: the same input passes sometimes and throws most of the time. In my environment (Node 22) a 2‑sheet workbook fails ~90% of reads.
### Environment
- exceljs 4.4.0 (same code on `master`)
- Node v22 (also reproduces on v18/v20); does not reproduce as often on older Node
- unzipper 0.10.14 (the range exceljs pins, `^0.10.11`)
### Minimal, self‑contained repro
```js
const ExcelJS = require('exceljs');
const {PassThrough} = require('stream');
(async () => {
const wb = new ExcelJS.Workbook();
wb.addWorksheet('First').addRow(['a', 'b']);
wb.addWorksheet('Second').addRow(['c', 'd']);
const buffer = await wb.xlsx.writeBuffer();
let ok = 0;
let failed = 0;
for (let i = 0; i < 20; i += 1) {
const stream = new PassThrough();
stream.end(Buffer.from(buffer));
const reader = new ExcelJS.stream.xlsx.WorkbookReader(stream, {});
try {
for await (const ws of reader) {
for await (const row of ws) { void row; }
}
ok += 1;
} catch (e) {
failed += 1;
if (failed === 1) console.error(e.message);
}
}
console.log(`ok=${ok} failed=${failed} of 20`);
})();
```
Observed on Node 22: `ok=2 failed=18 of 20` (varies run to run). Reading from a file path via `WorkbookReader(path)` reproduces it too.
### Root cause
`WorkbookReader.parse()` reads every part of the archive from a single streaming pass (`unzip.Parse({forceStream: true})`), which emits entries in **stored order**. `this.model` (the workbook model, needed to resolve sheet names) is set only when `xl/workbook.xml` is parsed by `_parseWorkbook`.
Two things combine:
1. exceljs's own writer stores `xl/workbook.xml` **last** in the archive (after the worksheets, sharedStrings and styles).
2. The single streaming pass does not reliably deliver that final entry to `_parseWorkbook` before the (deferred) worksheets are parsed under async iteration on Node ≥ 18.
So `_parseWorkbook` frequently hasn't run when a worksheet is parsed → `this.model` is `undefined` → `_parseWorksheet` throws on `this.model.sheets`. This lines up with the existing comment in `spec/integration/workbook-xlsx-reader.spec.js` (*"need some architectural changes to make stream read work properly because … shared strings, sheet names, etc are not read in guaranteed order"*).
For what it's worth, driving `unzipper.Parse({forceStream: true})` directly with a fully‑drained async iterator delivers all entries reliably — the loss shows up with the reader's interleaved consumption, so this is an exceljs‑side issue rather than a plain unzipper bug.
### Fix
Reading the archive through its **central directory** (`unzipper.Open`) instead of a single streaming pass makes every part addressable, so `rels`/`workbook`/`sharedStrings`/`styles` are always parsed before any worksheet. Worksheets are still consumed lazily (row‑by‑row), so the workbook is never materialized in memory. I'll open a PR with this change plus a regression test.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with lib/stream/xlsx/workbook-reader.js, especially parse(), _parseWorkbook(), and _parseWorksheet(), then read the related ordering note in spec/integration/workbook-xlsx-reader.spec.js. Run the provided two-sheet repro on Node 18 or newer and add a regression test showing repeated streaming reads complete without the undefined-model error while worksheet rows remain lazily consumable.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, nodejs
- Domain
- data
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 65/100