stream-chopper - npm Package Compare versions

Comparing version 2.2.1 to 3.0.0

index.js

 'use strict'
 
 const util = require('util')
+const zlib = require('zlib')
 const { Writable, PassThrough } = require('readable-stream')
@@ -42,3 +43,2 @@
   this._draining = false
-  this._destroyed = false
 
@@ -53,3 +53,6 @@ this._onunlock = null
   function oneos (err) {
-    if (err) self.emit('error', err)
+    // If oneos is called because of a close event, some streams will give a
+    // boolean as the first argument indicating if the stream was closed
+    // because of an error
+    if (err && typeof err !== 'boolean') self.destroy(err) // TODO: This error is already emitted by the stream. Maybe we shouldn't pass it on to destroy?
     self._removeStream()
@@ -67,3 +70,3 @@ }
 StreamChopper.prototype.chop = function (cb) {
-  if (this._destroyed) {
+  if (this.destroyed) {
     if (cb) process.nextTick(cb)
@@ -82,3 +85,3 @@ } else if (this._onunlock === null) {
 StreamChopper.prototype._startStream = function (cb) {
-  if (this._destroyed) return
+  if (this.destroyed) return
   if (this._locked) {
@@ -151,3 +154,3 @@ this._onunlock = cb
   }
-  if (this.time !== -1 && !this._destroyed && this._stream) {
+  if (this.time !== -1 && !this.destroyed && this._stream) {
     this._timer = setTimeout(() => {
@@ -162,3 +165,3 @@ this._timer = null
 StreamChopper.prototype._endStream = function (cb) {
-  if (this._destroyed) return
+  if (this.destroyed) return
   if (this._stream === null) {
@@ -198,3 +201,2 @@ if (cb) process.nextTick(cb)
 StreamChopper.prototype._write = function (chunk, enc, cb) {
-  if (this._destroyed) return
   if (this._stream === null) {
@@ -207,5 +209,6 @@ this._startStream(() => {
 
-  const destroyed = this._stream._writableState.destroyed || this._stream._readableState.destroyed
-
-  if (destroyed) {
+  // This guard is to protect against writes that happen in the same tick after
+  // a user destroys the stream. If it wasn't here, we'd accidentally write to
+  // the stream and it would emit an error
+  if (isDestroyed(this._stream)) {
     this._startStream(() => {
@@ -266,7 +269,2 @@ this._write(chunk, enc, cb)
 StreamChopper.prototype._destroy = function (err, cb) {
-  if (this._destroyed) return
-  this._destroyed = true
-
-  if (err) this.emit('error', err)
-
   const stream = this._stream
@@ -276,29 +274,8 @@ this._removeStream()
   if (stream !== null) {
-    stream.once('close', () => {
-      this.emit('close')
-      cb()
+    if (stream.destroyed === true) return cb(err)
+    destroyStream(stream, function () {
+      cb(err)
     })
-
-    switch (stream.constructor.name) {
-      case 'Gzip':
-      case 'Gunzip':
-      case 'Deflate':
-      case 'Inflate':
-        // In case stream is a zlib stream, these doesn't have a destroy function
-        // in Node.js 6. On top of that simply calling destroy on a zlib stream in
-        // Node.js 8+ will result in a memory leak. So until that is fixed, we need
-        // to call both close AND destroy.
-        //
-        // PR: https://github.com/nodejs/node/pull/23734
-        if (typeof stream.close === 'function') stream.close()
-        if (typeof stream.destroy === 'function') stream.destroy()
-        break
-      default:
-        // For other streams we assume calling just one of them is enough.
-        if (typeof stream.destroy === 'function') stream.destroy()
-        else if (typeof stream.emit === 'function') stream.emit('close')
-    }
   } else {
-    this.emit('close')
-    cb()
+    cb(err)
   }
@@ -308,3 +285,2 @@ }
 StreamChopper.prototype._final = function (cb) {
-  if (this._destroyed) return
   if (this._stream === null) return cb()
@@ -322,1 +298,52 @@ this._stream.end(cb)
 }
+
+// TODO: Make this work with all Node.js 6 streams. A Node.js 6 stream doesn't
+// have a destroyed flag because it doesn't have a .destroy() function. If the
+// stream is a zlib stream it will however have a _handle, which will be null
+// if the stream has been closed. We can check for that, but that coveres only
+// zlib streams
+function isDestroyed (stream) {
+  return stream.destroyed === true || stream._handle === null
+}
+
+function destroyStream (stream, cb) {
+  const emitClose = stream._writableState.emitClose
+  if (emitClose) stream.once('close', cb)
+
+  if (stream instanceof zlib.Gzip ||
+      stream instanceof zlib.Gunzip ||
+      stream instanceof zlib.Deflate ||
+      stream instanceof zlib.DeflateRaw ||
+      stream instanceof zlib.Inflate ||
+      stream instanceof zlib.InflateRaw ||
+      stream instanceof zlib.Unzip) {
+    // Zlib streams doesn't have a destroy function in Node.js 6. On top of
+    // that simply calling destroy on a zlib stream in Node.js 8+ will result
+    // in a memory leak as the handle isn't closed (an operation normally done
+    // by calling close). So until that is fixed, we need to manually close the
+    // handle after destroying the stream.
+    //
+    // PR: https://github.com/nodejs/node/pull/23734
+    if (typeof stream.destroy === 'function') {
+      // Manually close the stream instead of calling `close()` as that would
+      // have emitted 'close' again when calling `destroy()`
+      if (stream._handle && typeof stream._handle.close === 'function') {
+        stream._handle.close()
+        stream._handle = null
+      }
+
+      stream.destroy()
+    } else if (typeof stream.close === 'function') {
+      stream.close()
+    }
+  } else {
+    // For other streams we assume calling destroy is enough
+    if (typeof stream.destroy === 'function') stream.destroy()
+    // Or if there's no destroy (which Node.js 6 will not have on regular
+    // streams), emit `close` as that should trigger almost the same effect
+    else if (typeof stream.emit === 'function') stream.emit('close')
+  }
+
+  // In case this stream doesn't emit 'close', just call the callback manually
+  if (!emitClose) cb()
+}

package.json

 {
   "name": "stream-chopper",
-  "version": "2.2.1",
+  "version": "3.0.0",
   "description": "Chop a single stream of data into a series of readable streams",
   "main": "index.js",
   "dependencies": {
-    "readable-stream": "^2.3.6"
+    "readable-stream": "^3.0.6"
   },
@@ -47,5 +47,5 @@ "devDependencies": {
   "coordinates": [
-    55.778275,
+    55.778278,
     12.593052
   ]
 }

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

18500

increased by9.25%

Lines of code

280

increased by10.24%

Dependency changes

• + Addedreadable-stream@3.6.2(transitive)

• + Addedsafe-buffer@5.2.1(transitive)

• + Addedstring_decoder@1.3.0(transitive)

• - Removedcore-util-is@1.0.3(transitive)

• - Removedisarray@1.0.0(transitive)

• - Removedprocess-nextick-args@2.0.1(transitive)

• - Removedreadable-stream@2.3.8(transitive)

• - Removedsafe-buffer@5.1.2(transitive)

• - Removedstring_decoder@1.1.1(transitive)

• Updatedreadable-stream@^3.0.6