[skywalking-nodejs] branch master updated: [Enhancement] tweaks for better error handling (#5)

[ke...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

kezhenxu94 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking-nodejs.git


The following commit(s) were added to refs/heads/master by this push:
     new 92399c3  [Enhancement] tweaks for better error handling (#5)
92399c3 is described below

commit 92399c37f5ce1ce65a2d94d51e7706d1165e88ab
Author: Tomasz Pytel <to...@gmail.com>
AuthorDate: Wed Dec 2 23:32:48 2020 -0300

    [Enhancement] tweaks for better error handling (#5)
---
 src/plugins/HttpPlugin.ts           | 66 ++++++++++++++++++++-----------------
 src/trace/context/ContextManager.ts | 17 ++--------
 2 files changed, 37 insertions(+), 46 deletions(-)

diff --git a/src/plugins/HttpPlugin.ts b/src/plugins/HttpPlugin.ts
index 3b5b171..3d8f569 100644
--- a/src/plugins/HttpPlugin.ts
+++ b/src/plugins/HttpPlugin.ts
@@ -60,44 +60,49 @@ class HttpPlugin implements SwPlugin {
               };
         const operation = pathname.replace(/\?.*$/g, '');
 
-        const [span, request]: [Span, ClientRequest] = ContextManager.withSpanNoStop(
-            ContextManager.current.newExitSpan(operation, host), (_span: Span, args) => {
+        let stopped = 0;  // compensating if request aborted right after creation 'close' is not emitted
+        const span = ContextManager.current.newExitSpan(operation, host).start();
+        const stopIfNotStopped = () => !stopped++ ? span.stop() : null;  // make sure we stop only once
 
-          _span.component = Component.HTTP;
-          _span.layer = SpanLayer.HTTP;
-          _span.tag(Tag.httpURL(host + pathname));
+        try {
+          span.component = Component.HTTP;
+          span.layer = SpanLayer.HTTP;
+          span.tag(Tag.httpURL(host + pathname));
 
-          const _request = original.apply(this, args);
+          const request: ClientRequest = original.apply(this, arguments);
 
-          _span.extract().items.forEach((item) => {
-            _request.setHeader(item.key, item.value);
+          span.extract().items.forEach((item) => {
+            request.setHeader(item.key, item.value);
           });
 
-          return [_span, _request];
-
-        }, arguments);
+          request.on('close', stopIfNotStopped);
+          request.on('abort', () => (span.errored = true, stopIfNotStopped()));
+          request.on('error', (err) => (span.error(err), stopIfNotStopped()));
+
+          request.prependListener('response', (res) => {
+            span.resync();
+            span.tag(Tag.httpStatusCode(res.statusCode));
+            if (res.statusCode && res.statusCode >= 400) {
+              span.errored = true;
+            }
+            if (res.statusMessage) {
+              span.tag(Tag.httpStatusMsg(res.statusMessage));
+            }
+            stopIfNotStopped();
+          });
 
-        span.async();
+          span.async();
 
-        let stopped = 0;  // compensating if request aborted right after creation 'close' is not emitted
-        const stopIfNotStopped = () => !stopped++ ? span.stop() : null;  // make sure we stop only once
-        request.on('close', stopIfNotStopped);
-        request.on('abort', () => (span.errored = true, stopIfNotStopped()));
-        request.on('error', (err) => (span.error(err), stopIfNotStopped()));
+          return request;
 
-        request.prependListener('response', (res) => {
-          span.resync();
-          span.tag(Tag.httpStatusCode(res.statusCode));
-          if (res.statusCode && res.statusCode >= 400) {
-            span.errored = true;
+        } catch (e) {
+          if (!stopped) {
+            span.error(e);
+            stopIfNotStopped();
           }
-          if (res.statusMessage) {
-            span.tag(Tag.httpStatusMsg(res.statusMessage));
-          }
-          stopIfNotStopped();
-        });
 
-        return request;
+          throw e;
+        }
       };
     })(http.request);
   }
@@ -122,10 +127,9 @@ class HttpPlugin implements SwPlugin {
 
         const carrier = ContextCarrier.from(headersMap);
         const operation = (req.url || '/').replace(/\?.*/g, '');
+        const span = ContextManager.current.newEntrySpan(operation, carrier);
 
-        return ContextManager.withSpan(ContextManager.current.newEntrySpan(operation, carrier),
-            (span, self, args) => {
-
+        return ContextManager.withSpan(span, (self, args) => {
           span.component = Component.HTTP_SERVER;
           span.layer = SpanLayer.HTTP;
           span.tag(Tag.httpURL((req.headers.host || '') + req.url));
diff --git a/src/trace/context/ContextManager.ts b/src/trace/context/ContextManager.ts
index 7f373a6..6fd34e6 100644
--- a/src/trace/context/ContextManager.ts
+++ b/src/trace/context/ContextManager.ts
@@ -68,7 +68,7 @@ class ContextManager {
       span.start();
 
     try {
-      return callback(span, ...args);
+      return callback(...args);
     } catch (e) {
       span.error(e);
       throw e;
@@ -82,7 +82,7 @@ class ContextManager {
       span.start();
 
     try {
-      return await callback(span, ...args);
+      return await callback(...args);
     } catch (e) {
       span.error(e);
       throw e;
@@ -90,19 +90,6 @@ class ContextManager {
       span.stop();
     }
   }
-
-  withSpanNoStop(span: Span, callback: (...args: any[]) => any, ...args: any[]): any {
-    if(!span.startTime)
-      span.start();
-
-    try {
-      return callback(span, ...args);
-    } catch (e) {
-      span.error(e);
-      span.stop();
-      throw e;
-    }
-  }
 }
 
 export default new ContextManager();