From dd0db287cde105e33a5af8119a975a594939ddb6 Mon Sep 17 00:00:00 2001 From: Mike Marcacci Date: Sun, 18 Apr 2021 11:42:31 -0700 Subject: [PATCH 1/5] allow specification of custom tmpdir --- src/index.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index f4d7095..e227a2d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -83,6 +83,7 @@ export class ReadStream extends Readable { export interface WriteStreamOptions { highWaterMark?: WritableOptions["highWaterMark"]; defaultEncoding?: WritableOptions["defaultEncoding"]; + tmpdir?: () => string; } export class WriteStream extends Writable { @@ -106,7 +107,10 @@ export class WriteStream extends Writable { return; } - this._path = join(tmpdir(), `capacitor-${buffer.toString("hex")}.tmp`); + this._path = join( + (options?.tmpdir ?? tmpdir)(), + `capacitor-${buffer.toString("hex")}.tmp` + ); // Create a file in the OS's temporary files directory. open(this._path, "wx+", 0o600, (error, fd) => { From 62ab9e871ef51de04f76aa2044cecdc16a302cb5 Mon Sep 17 00:00:00 2001 From: Mike Marcacci Date: Sun, 18 Apr 2021 12:05:26 -0700 Subject: [PATCH 2/5] Add test to reproduce init failure (see #45) --- src/index.test.ts | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/index.test.ts b/src/index.test.ts index 1c54e68..57bfeed 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -76,6 +76,29 @@ test("Data from a complete stream.", async (t) => { ); }); +test("Error while initializing.", async (t) => { + // Create a new capacitor + const capacitor1 = new WriteStream({ tmpdir: () => "/tmp/does-not-exist" }); + + let resolve: () => void, reject: (error: Error) => void; + const promise = new Promise((_resolve, _reject) => { + resolve = _resolve; + reject = _reject; + }); + + // Synchronously attach an error listener. + capacitor1.on("error", (error) => { + try { + t.is((error as any).code, "ENOENT"); + resolve(); + } catch (error) { + reject(error); + } + }); + + await promise; +}); + test("Allows specification of encoding in createReadStream.", async (t) => { const data = Buffer.from("1".repeat(10), "utf8"); const source = new Readable({ From 8fc45146e4fd9b073ee92534d7ffbd09d5a2620e Mon Sep 17 00:00:00 2001 From: Mike Marcacci Date: Sun, 18 Apr 2021 12:06:04 -0700 Subject: [PATCH 3/5] Fix #45 - correctly handle init failures --- src/index.ts | 62 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 20 deletions(-) diff --git a/src/index.ts b/src/index.ts index e227a2d..60bfaca 100644 --- a/src/index.ts +++ b/src/index.ts @@ -128,6 +128,32 @@ export class WriteStream extends Writable { }); } + _cleanup = (callback: (error: null | Error) => void): void => { + const fd = this._fd; + const path = this._path; + + if (typeof fd !== "number" || typeof path !== "string") { + callback(null); + return; + } + + // Close the file descriptor. + close(fd, (closeError) => { + // An error here probably means the fd was already closed, but we can + // still try to unlink the file. + unlink(path, (unlinkError) => { + // If we are unable to unlink the file, the operating system will + // clean up on next restart, since we use store thes in `os.tmpdir()` + this._fd = null; + + // We avoid removing this until now in case an exit occurs while + // asyncronously cleaning up. + processExitProxy.off("exit", this._cleanupSync); + callback(unlinkError ?? closeError); + }); + }); + }; + _cleanupSync = (): void => { processExitProxy.off("exit", this._cleanupSync); @@ -195,32 +221,28 @@ export class WriteStream extends Writable { error: undefined | null | Error, callback: (error?: null | Error) => any ): void { - const fd = this._fd; - const path = this._path; - if (typeof fd !== "number" || typeof path !== "string") { - this.once("ready", () => this._destroy(error, callback)); - return; + // Destroy all attached read streams. + for (const readStream of this._readStreams) { + readStream.destroy(error || undefined); } - // Close the file descriptor. - close(fd, (closeError) => { - // An error here probably means the fd was already closed, but we can - // still try to unlink the file. - unlink(path, (unlinkError) => { - // If we are unable to unlink the file, the operating system will - // clean up on next restart, since we use store thes in `os.tmpdir()` - this._fd = null; + // This capacitor is fully initialized. + if (typeof this._fd === "number" && typeof this._path === "string") { + this._cleanup((cleanupError) => callback(cleanupError ?? error)); + return; + } - // We avoid removing this until now in case an exit occurs while - // asyncronously cleaning up. - processExitProxy.off("exit", this._cleanupSync); - callback(unlinkError || closeError || error); + // This capacitor has not yet finished initialization; if initialization + // does complete, immediately clean up after. + this.once("ready", () => { + this._cleanup((cleanupError) => { + if (cleanupError) { + this.emit("error", cleanupError); + } }); }); - // Destroy all attached read streams. - for (const readStream of this._readStreams) - readStream.destroy(error || undefined); + callback(error); } createReadStream(options?: ReadStreamOptions): ReadStream { From 5b84654b8b147cd5b557104334bceb1eec4ac743 Mon Sep 17 00:00:00 2001 From: Mike Marcacci Date: Sun, 18 Apr 2021 12:13:52 -0700 Subject: [PATCH 4/5] add fix to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0db07d6..ef3e9b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -95,6 +95,7 @@ - Upgrade dependencies. - Prevent Node.js max listeners exceeded warnings if many `fs-capacitor` `ReadStream` instances are created at the same time, fixing [#30](https://github.com/mike-marcacci/fs-capacitor/issues/30) via [#42](https://github.com/mike-marcacci/fs-capacitor/pull/42). +- Ensure initialization failures are reported, fixing [#45](https://github.com/mike-marcacci/fs-capacitor/issues/45) via [#46](https://github.com/mike-marcacci/fs-capacitor/pull/46/files). - **BREAKING:** Drop support for node 13. - **BREAKING:** Drop support for node 10. - **BREAKING:** Change module type to ES module. From e5cd25a2d71847bbd44e7deb9cf222e384a7b21d Mon Sep 17 00:00:00 2001 From: Mike Marcacci Date: Sun, 18 Apr 2021 12:17:46 -0700 Subject: [PATCH 5/5] document new tmpdir option --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index f48d125..90e280f 100644 --- a/README.md +++ b/README.md @@ -106,6 +106,10 @@ Uses node's default of `16384` (16kb). Optional buffer size at which the writabl Uses node's default of `utf8`. Optional default encoding to use when no encoding is specified as an argument to `stream.write()`. See [node's docs for `stream.Writable`](https://nodejs.org/api/stream.html#stream_constructor_new_stream_writable_options). Possible values depend on the version of node, and are [defined in node's buffer implementation](https://github.com/nodejs/node/blob/master/lib/buffer.js); +#### `.tmpdir` + +Used node's [`os.tmpdir`](https://nodejs.org/api/os.html#os_os_tmpdir) by default. This function returns the directory used by fs-capacitor to store file buffers, and is intended primarily for testing and debugging. + ### ReadStream `ReadStream` extends [`stream.Readable`](https://nodejs.org/api/stream.html#stream_new_stream_readable_options);