squash! follow an alternative approach

Throw an Error synchronously instead of fiddling with 'error' events.
nodejs · May 18, 2017 · c5440af · c5440af
1 parent 7d78533
commit c5440af
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 46 deletions.
diff --git a/doc/api/zlib.md b/doc/api/zlib.md
@@ -437,6 +437,10 @@ added: v0.5.8
 
 Returns a new [DeflateRaw][] object with an [options][].
 
+**Note:** zlib library rejects requests for 256-byte windows (i.e.,
+`{ windowBits: 8 }` in `options`). An `Error` will be thrown when creating
+a [DeflateRaw][] object with this specific value of the `windowBits` option.
+
 ## zlib.createGunzip([options])
 <!-- YAML
 added: v0.5.8

diff --git a/lib/zlib.js b/lib/zlib.js
@@ -146,14 +146,7 @@ function zlibOnError(message, errno) {
   var error = new Error(message);
   error.errno = errno;
   error.code = codes[errno];
-
-  if (this._initSuccess) {
-    this.emit('error', error);
-  } else {
-    process.nextTick(() => {
-      this.emit('error', error);
-    });
-  }
+  this.emit('error', error);
 }
 
 function flushCallback(level, strategy, callback) {
@@ -229,7 +222,6 @@ class Zlib extends Transform {
     this._handle = new binding.Zlib(mode);
     this._handle.onerror = zlibOnError.bind(this);
     this._hadError = false;
-    this._initSuccess = false;
 
     var level = constants.Z_DEFAULT_COMPRESSION;
     if (typeof opts.level === 'number') level = opts.level;
@@ -243,18 +235,13 @@ class Zlib extends Transform {
     var memLevel = constants.Z_DEFAULT_MEMLEVEL;
     if (typeof opts.memLevel === 'number') memLevel = opts.memLevel;
 
-    this._initSuccess = this._handle.init(windowBits,
-                                          level,
-                                          memLevel,
-                                          strategy,
-                                          opts.dictionary);
-
-    if (this._initSuccess) {
-      this._buffer = Buffer.allocUnsafe(this._chunkSize);
-    } else {
-      this._buffer = null;
-    }
+    this._handle.init(windowBits,
+                      level,
+                      memLevel,
+                      strategy,
+                      opts.dictionary);
 
+    this._buffer = Buffer.allocUnsafe(this._chunkSize);
     this._offset = 0;
     this._level = level;
     this._strategy = strategy;
@@ -486,9 +473,7 @@ function _close(engine, callback) {
   if (!engine._handle)
     return;
 
-  if (engine._initSuccess)
-    engine._handle.close();
-
+  engine._handle.close();
   engine._handle = null;
 }
 

diff --git a/src/node_zlib.cc b/src/node_zlib.cc
@@ -40,7 +40,6 @@
 namespace node {
 
 using v8::Array;
-using v8::Boolean;
 using v8::Context;
 using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
@@ -481,11 +480,9 @@ class ZCtx : public AsyncWrap {
       memcpy(dictionary, Buffer::Data(dictionary_), dictionary_len);
     }
 
-    bool result = Init(ctx, level, windowBits, memLevel, strategy,
-                       dictionary, dictionary_len);
+    Init(ctx, level, windowBits, memLevel, strategy,
+         dictionary, dictionary_len);
     SetDictionary(ctx);
-
-    args.GetReturnValue().Set(Boolean::New(args.GetIsolate(), result));
   }
 
   static void Params(const FunctionCallbackInfo<Value>& args) {
@@ -502,7 +499,7 @@ class ZCtx : public AsyncWrap {
     SetDictionary(ctx);
   }
 
-  static bool Init(ZCtx *ctx, int level, int windowBits, int memLevel,
+  static void Init(ZCtx *ctx, int level, int windowBits, int memLevel,
                    int strategy, char* dictionary, size_t dictionary_len) {
     ctx->level_ = level;
     ctx->windowBits_ = windowBits;
@@ -554,21 +551,19 @@ class ZCtx : public AsyncWrap {
         CHECK(0 && "wtf?");
     }
 
-    if (ctx->err_ != Z_OK) {
-      ZCtx::Error(ctx, "Init error");
-      if (dictionary != nullptr)
-        delete[] dictionary;
-      return false;
-    }
-
-
     ctx->dictionary_ = reinterpret_cast<Bytef *>(dictionary);
     ctx->dictionary_len_ = dictionary_len;
 
     ctx->write_in_progress_ = false;
     ctx->init_done_ = true;
 
-    return true;
+    if (ctx->err_ != Z_OK) {
+      if (dictionary != nullptr) {
+        delete[] dictionary;
+        ctx->dictionary_ = nullptr;
+      }
+      ctx->env()->ThrowError("Init error");
+    }
   }
 
   static void SetDictionary(ZCtx* ctx) {

diff --git a/test/parallel/test-zlib-failed-init.js b/test/parallel/test-zlib-failed-init.js
@@ -1,15 +1,13 @@
 'use strict';
 
-const common = require('../common');
+require('../common');
 
 const assert = require('assert');
 const zlib = require('zlib');
 
-// For raw deflate or gzip encoding, a request for a 256-byte window is
-// rejected as invalid, since only zlib headers provide means of transmitting
-// the window size to the decompressor.
+// For raw deflate encoding, requests for 256-byte windows are rejected as
+// invalid by zlib.
 // (http://zlib.net/manual.html#Advanced)
-const deflate = zlib.createDeflateRaw({ windowBits: 8 });
-deflate.on('error', common.mustCall((error) => {
-  assert.ok(/Init error/.test(error));
-}));
+assert.throws(() => {
+  zlib.createDeflateRaw({ windowBits: 8 });
+}, /Init error/);