From 016876413a2a88a8b574ab578a5f0328fd5b274e Mon Sep 17 00:00:00 2001
From: cgewecke <christophergewecke@gmail.com>
Date: Mon, 16 Dec 2019 21:50:52 -0800
Subject: [PATCH 1/9] Add tests for core-subscriptions edge cases

---
 packages/web3-core-subscriptions/src/index.js |  2 +-
 .../src/subscription.js                       |  6 +-
 test/eth.subscribe.ganache.js                 | 56 +++++++++++++++++++
 3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/packages/web3-core-subscriptions/src/index.js b/packages/web3-core-subscriptions/src/index.js
index e12811af8e5..113f6757fd9 100644
--- a/packages/web3-core-subscriptions/src/index.js
+++ b/packages/web3-core-subscriptions/src/index.js
@@ -59,7 +59,7 @@ Subscriptions.prototype.buildCall = function() {
         }
 
         var subscription = new Subscription({
-            subscription: _this.subscriptions[arguments[0]],
+            subscription: _this.subscriptions[arguments[0]] || {}, // Subscript might not exist
             requestManager: _this.requestManager,
             type: _this.type
         });
diff --git a/packages/web3-core-subscriptions/src/subscription.js b/packages/web3-core-subscriptions/src/subscription.js
index 65310126f4a..59b4e25e746 100644
--- a/packages/web3-core-subscriptions/src/subscription.js
+++ b/packages/web3-core-subscriptions/src/subscription.js
@@ -77,7 +77,11 @@ Subscription.prototype._validateArgs = function (args) {
         subscription.params = 0;
 
     if (args.length !== subscription.params) {
-        throw errors.InvalidNumberOfParams(args.length, subscription.params + 1, args[0]);
+        throw errors.InvalidNumberOfParams(
+            args.length,
+            subscription.params,
+            subscription.subscriptionName
+        );
     }
 };
 
diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js
index cde97b6c2db..354bff9429e 100644
--- a/test/eth.subscribe.ganache.js
+++ b/test/eth.subscribe.ganache.js
@@ -35,6 +35,62 @@ describe('subscription connect/reconnect', function() {
         });
     });
 
+    it('subscribes with a callback', function(){
+        let subscription;
+
+        return new Promise(async function (resolve) {
+            subscription = web3.eth
+                .subscribe('newBlockHeaders', function(err, result){
+                    assert(result.parentHash);
+                    subscription.unsubscribe(); // Stop listening..
+                    resolve();
+            });
+         })
+    });
+
+    it('resubscribes', function(){
+        let subscription;
+
+        return new Promise(async function (resolve) {
+            subscription = web3.eth.subscribe('newBlockHeaders');
+            subscription.unsubscribe();
+            subscription.resubscribe()
+
+            subscription.on('data', function(result){
+                console.log(result.parentHash)
+                assert(result.parentHash);
+                subscription.unsubscribe(); // Stop listening..
+                resolve();
+            })
+         })
+    });
+
+    it('allows a subscription which does not exist', function(){
+        web3.eth.subscribe('subscription-does-not-exists');
+    });
+
+    it('errors when zero params subscrip. is called with the wrong arguments', function(){
+        try {
+            web3.eth.subscribe('newBlockHeaders', 5);
+            assert.fail();
+        } catch (err) {
+            assert(err.message.includes('Invalid number of parameters for "newHeads"'))
+            assert(err.message.includes('Got 1 expected 0'));
+        }
+    });
+
+    // Could not get the .on('error') version of this to work - maybe a race condition setting it up.
+    it('errors when the provider is not set', function(){
+        web3 = new Web3();
+
+        return new Promise(async function (resolve) {
+            web3.eth.subscribe('newBlockHeaders', function(err, result){
+                assert(err.message.includes('No provider set'));
+                resolve()
+            })
+        });
+    });
+
     it('errors when the `eth_subscribe` request got send, the reponse isnt returned from the node, and the connection does get closed in the mean time', async function() {
         await pify(server.close)();
 

From 6cf61ed02d0692be6d01c3e657a5f15a8ac8fb05 Mon Sep 17 00:00:00 2001
From: cgewecke <christophergewecke@gmail.com>
Date: Mon, 16 Dec 2019 22:35:27 -0800
Subject: [PATCH 2/9] Remove console.log in test

---
 test/eth.subscribe.ganache.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js
index 354bff9429e..c5923dbf970 100644
--- a/test/eth.subscribe.ganache.js
+++ b/test/eth.subscribe.ganache.js
@@ -57,7 +57,6 @@ describe('subscription connect/reconnect', function() {
             subscription.resubscribe()
 
             subscription.on('data', function(result){
-                console.log(result.parentHash)
                 assert(result.parentHash);
                 subscription.unsubscribe(); // Stop listening..
                 resolve();

From 4c6bab9939369e283cbd7688f1d00ea40c553a8a Mon Sep 17 00:00:00 2001
From: cgewecke <christophergewecke@gmail.com>
Date: Tue, 17 Dec 2019 01:52:52 -0800
Subject: [PATCH 3/9] Use done

---
 test/eth.subscribe.ganache.js | 69 +++++++++++++++--------------------
 1 file changed, 29 insertions(+), 40 deletions(-)

diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js
index c5923dbf970..763e75ec6c4 100644
--- a/test/eth.subscribe.ganache.js
+++ b/test/eth.subscribe.ganache.js
@@ -7,6 +7,7 @@ describe('subscription connect/reconnect', function() {
     let server;
     let web3;
     let accounts;
+    let subscription;
     const port = 8545;
 
     beforeEach(async function() {
@@ -24,44 +25,34 @@ describe('subscription connect/reconnect', function() {
         }
     });
 
-    it('subscribes (baseline)', function() {
-        return new Promise(async function (resolve) {
-            web3.eth
-                .subscribe('newBlockHeaders')
-                .on('data', function(result) {
-                    assert(result.parentHash);
-                    resolve();
-                });
-        });
+    it('subscribes (baseline)', function(done) {
+        web3.eth
+            .subscribe('newBlockHeaders')
+            .on('data', function(result) {
+                assert(result.parentHash);
+                done();
+            });
     });
 
-    it('subscribes with a callback', function(){
-        let subscription;
-
-        return new Promise(async function (resolve) {
-            subscription = web3.eth
-                .subscribe('newBlockHeaders', function(err, result){
-                    assert(result.parentHash);
-                    subscription.unsubscribe(); // Stop listening..
-                    resolve();
+    it('subscribes with a callback', function(done){
+        subscription = web3.eth
+            .subscribe('newBlockHeaders', function(err, result){
+                assert(result.parentHash);
+                subscription.unsubscribe(); // Stop listening..
+                done();
             });
-         })
     });
 
-    it('resubscribes', function(){
-        let subscription;
+    it('resubscribes', function(done){
+        subscription = web3.eth.subscribe('newBlockHeaders')
+        subscription.unsubscribe();
+        subscription.resubscribe()
 
-        return new Promise(async function (resolve) {
-            subscription = web3.eth.subscribe('newBlockHeaders');
-            subscription.unsubscribe();
-            subscription.resubscribe()
-
-            subscription.on('data', function(result){
-                assert(result.parentHash);
-                subscription.unsubscribe(); // Stop listening..
-                resolve();
-            })
-         })
+        subscription.on('data', function(result){
+            assert(result.parentHash);
+            subscription.unsubscribe(); // Stop listening..
+            done();
+        })
     });
 
     it('allows a subscription which does not exist', function(){
@@ -70,7 +61,7 @@ describe('subscription connect/reconnect', function() {
 
     it('errors when zero params subscrip. is called with the wrong arguments', function(){
         try {
-            web3.eth.subscribe('newBlockHeaders', 5);
+            web3.eth.subscribe('newBlockHeaders', 5)
             assert.fail();
         } catch (err) {
             assert(err.message.includes('Invalid number of parameters for "newHeads"'))
@@ -79,15 +70,13 @@ describe('subscription connect/reconnect', function() {
     });
 
     // Could not get the .on('error') version of this to work - maybe a race condition setting it up.
-    it('errors when the provider is not set', function(){
+    it('errors when the provider is not set', function(done){
         web3 = new Web3();
 
-        return new Promise(async function (resolve) {
-            web3.eth.subscribe('newBlockHeaders', function(err, result){
-                assert(err.message.includes('No provider set'));
-                resolve()
-            })
-        });
+        web3.eth.subscribe('newBlockHeaders', function(err, result){
+            assert(err.message.includes('No provider set'));
+            done();
+        })
     });
 
     it('errors when the `eth_subscribe` request got send, the reponse isnt returned from the node, and the connection does get closed in the mean time', async function() {

From 21d854c362475a694d79f32dc38dbfa4272e41f9 Mon Sep 17 00:00:00 2001
From: cgewecke <christophergewecke@gmail.com>
Date: Tue, 17 Dec 2019 02:05:21 -0800
Subject: [PATCH 4/9] Rewrite resubscribe test per review comment

---
 test/eth.subscribe.ganache.js | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js
index 763e75ec6c4..d111d74689b 100644
--- a/test/eth.subscribe.ganache.js
+++ b/test/eth.subscribe.ganache.js
@@ -44,15 +44,28 @@ describe('subscription connect/reconnect', function() {
     });
 
     it('resubscribes', function(done){
-        subscription = web3.eth.subscribe('newBlockHeaders')
-        subscription.unsubscribe();
-        subscription.resubscribe()
+        let stage = 0;
 
-        subscription.on('data', function(result){
-            assert(result.parentHash);
-            subscription.unsubscribe(); // Stop listening..
-            done();
-        })
+        subscription = web3.eth
+            .subscribe('newBlockHeaders')
+            .on('data', function(result) {
+                assert(result.parentHash);
+                subscription.unsubscribe();
+                stage = 1;
+            });
+
+        // Resubscribe from outside
+        interval = setInterval(async function(){
+            if (stage === 1){
+                clearInterval(interval);
+                subscription.resubscribe();
+                subscription.on('data', function(result){
+                    assert(result.parentHash);
+                    subscription.unsubscribe(); // Stop listening..
+                    done();
+                })
+            }
+        }, 500)
     });
 
     it('allows a subscription which does not exist', function(){

From 1cb8b8034a615a04b22855049762f04f3a6ac931 Mon Sep 17 00:00:00 2001
From: cgewecke <christophergewecke@gmail.com>
Date: Tue, 17 Dec 2019 02:19:53 -0800
Subject: [PATCH 5/9] Increase resubscribes timeout

---
 test/eth.subscribe.ganache.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js
index d111d74689b..fbfa6cba40c 100644
--- a/test/eth.subscribe.ganache.js
+++ b/test/eth.subscribe.ganache.js
@@ -3,7 +3,7 @@ const ganache = require('ganache-cli');
 const pify = require('pify');
 const Web3 = require('./helpers/test.utils').getWeb3();
 
-describe('subscription connect/reconnect', function() {
+describe.only('subscription connect/reconnect', function() {
     let server;
     let web3;
     let accounts;
@@ -44,6 +44,8 @@ describe('subscription connect/reconnect', function() {
     });
 
     it('resubscribes', function(done){
+        this.timeout(5000);
+
         let stage = 0;
 
         subscription = web3.eth

From 7790a26ac5a8d72d5a54e520e21e7bd97484c8b9 Mon Sep 17 00:00:00 2001
From: cgewecke <christophergewecke@gmail.com>
Date: Tue, 17 Dec 2019 02:29:30 -0800
Subject: [PATCH 6/9] Remove .only

---
 test/eth.subscribe.ganache.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js
index fbfa6cba40c..1604f63c347 100644
--- a/test/eth.subscribe.ganache.js
+++ b/test/eth.subscribe.ganache.js
@@ -3,7 +3,7 @@ const ganache = require('ganache-cli');
 const pify = require('pify');
 const Web3 = require('./helpers/test.utils').getWeb3();
 
-describe.only('subscription connect/reconnect', function() {
+describe('subscription connect/reconnect', function() {
     let server;
     let web3;
     let accounts;

From ac880f656b564bf275ff2d417c3f2d62830a9ea2 Mon Sep 17 00:00:00 2001
From: cgewecke <christophergewecke@gmail.com>
Date: Tue, 17 Dec 2019 12:24:06 -0800
Subject: [PATCH 7/9] Add resubscribe to existing subscription case

---
 test/eth.subscribe.ganache.js | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js
index 1604f63c347..380eb629d47 100644
--- a/test/eth.subscribe.ganache.js
+++ b/test/eth.subscribe.ganache.js
@@ -43,7 +43,27 @@ describe('subscription connect/reconnect', function() {
             });
     });
 
-    it('resubscribes', function(done){
+    it('resubscribes to an existing subscription', function(done){
+        this.timeout(5000);
+
+        let stage = 0;
+
+        subscription = web3.eth.subscribe('newBlockHeaders');
+
+        subscription.on('data', function(result){
+            if (stage === 0) {
+              subscription.resubscribe();
+              stage = 1;
+              return;
+            }
+
+            assert(result.parentHash);
+            subscription.unsubscribe(); // Stop listening..
+            done();
+        });
+    });
+
+    it('resubscribes after being unsubscribed', function(done){
         this.timeout(5000);
 
         let stage = 0;

From d21d746d2567f0cf7adc6bf699404d9bae3bf11b Mon Sep 17 00:00:00 2001
From: cgewecke <christophergewecke@gmail.com>
Date: Thu, 19 Dec 2019 10:07:07 -0800
Subject: [PATCH 8/9] Run initially emitted errors in a zero-length timeout
 (plus tests)

---
 .../src/subscription.js                       | 22 ++++++++----
 test/eth.subscribe.ganache.js                 | 34 +++++++++++++++++--
 2 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/packages/web3-core-subscriptions/src/subscription.js b/packages/web3-core-subscriptions/src/subscription.js
index 59b4e25e746..c47c8fd72aa 100644
--- a/packages/web3-core-subscriptions/src/subscription.js
+++ b/packages/web3-core-subscriptions/src/subscription.js
@@ -196,18 +196,28 @@ Subscription.prototype.subscribe = function() {
         return this;
     }
 
+    // throw error, if provider is not set
     if(!this.options.requestManager.provider) {
-        var err1 = new Error('No provider set.');
-        this.callback(err1, null, this);
-        this.emit('error', err1);
+        setTimeout(function(){
+            var err1 = new Error('No provider set.');
+            _this.callback(err1, null, _this);
+            _this.emit('error', err1);
+        },0);
+
         return this;
     }
 
     // throw error, if provider doesnt support subscriptions
     if(!this.options.requestManager.provider.on) {
-        var err2 = new Error('The current provider doesn\'t support subscriptions: '+ this.options.requestManager.provider.constructor.name);
-        this.callback(err2, null, this);
-        this.emit('error', err2);
+        setTimeout(function(){
+            var err2 = new Error(
+                'The current provider doesn\'t support subscriptions: ' +
+                _this.options.requestManager.provider.constructor.name
+            );
+            _this.callback(err2, null, _this);
+            _this.emit('error', err2);
+        },0);
+
         return this;
     }
 
diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js
index 380eb629d47..2f86d8afc3f 100644
--- a/test/eth.subscribe.ganache.js
+++ b/test/eth.subscribe.ganache.js
@@ -104,8 +104,7 @@ describe('subscription connect/reconnect', function() {
         }
     });
 
-    // Could not get the .on('error') version of this to work - maybe a race condition setting it up.
-    it('errors when the provider is not set', function(done){
+    it('errors when the provider is not set (callback)', function(done){
         web3 = new Web3();
 
         web3.eth.subscribe('newBlockHeaders', function(err, result){
@@ -114,6 +113,37 @@ describe('subscription connect/reconnect', function() {
         })
     });
 
+    it('errors when the provider is not set (.on("error"))', function(done){
+        web3 = new Web3();
+
+        web3.eth
+            .subscribe('newBlockHeaders')
+            .on("error", function(err){
+                assert(err.message.includes('No provider set'));
+                done();
+            })
+    });
+
+    it('errors when the provider does not support subscriptions (callback)', function(done){
+        web3 = new Web3('http://localhost:' + port);
+
+        web3.eth.subscribe('newBlockHeaders', function(err, result){
+            assert(err.message.includes("provider doesn't support subscriptions: HttpProvider"));
+            done();
+        });
+    });
+
+    it('errors when the provider is not set (.on("error"))', function(done){
+        web3 = new Web3('http://localhost:' + port);
+
+        web3.eth
+            .subscribe('newBlockHeaders')
+            .on("error", function(err){
+                assert(err.message.includes("provider doesn't support subscriptions: HttpProvider"));
+                done();
+            })
+    });
+
     it('errors when the `eth_subscribe` request got send, the reponse isnt returned from the node, and the connection does get closed in the mean time', async function() {
         await pify(server.close)();
 

From 9038a7a9d4a6872234302d933bb6517a26052c2b Mon Sep 17 00:00:00 2001
From: cgewecke <christophergewecke@gmail.com>
Date: Mon, 6 Jan 2020 12:42:24 -0800
Subject: [PATCH 9/9] Wrap more immediate errors in 0 length timeouts

---
 packages/web3-core-subscriptions/src/subscription.js | 12 ++++++++----
 test/eth.subscribe.ganache.js                        | 10 ++++++++++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/packages/web3-core-subscriptions/src/subscription.js b/packages/web3-core-subscriptions/src/subscription.js
index c47c8fd72aa..dc44cf4c613 100644
--- a/packages/web3-core-subscriptions/src/subscription.js
+++ b/packages/web3-core-subscriptions/src/subscription.js
@@ -246,8 +246,10 @@ Subscription.prototype.subscribe = function() {
                 // TODO subscribe here? after the past logs?
 
             } else {
-                _this.callback(err, null, _this);
-                _this.emit('error', err);
+                setTimeout(function(){
+                    _this.callback(err, null, _this);
+                    _this.emit('error', err);
+                },0);
             }
         });
     }
@@ -289,8 +291,10 @@ Subscription.prototype.subscribe = function() {
                 }
             });
         } else {
-            _this.callback(err, false, _this);
-            _this.emit('error', err);
+            setTimeout(function(){
+                _this.callback(err, false, _this);
+                _this.emit('error', err);
+            },0);
         }
     });
 
diff --git a/test/eth.subscribe.ganache.js b/test/eth.subscribe.ganache.js
index 2f86d8afc3f..89a0d7c94dd 100644
--- a/test/eth.subscribe.ganache.js
+++ b/test/eth.subscribe.ganache.js
@@ -43,6 +43,16 @@ describe('subscription connect/reconnect', function() {
             });
     });
 
+    it('subscription emits a connected event', function(done){
+        subscription = web3.eth
+            .subscribe('newBlockHeaders')
+            .on('connected', function(result){
+                assert(result)              // First subscription
+                subscription.unsubscribe(); // Stop listening..
+                done();
+            });
+    });
+
     it('resubscribes to an existing subscription', function(done){
         this.timeout(5000);