commit 92399c37f5ce1ce65a2d94d51e7706d1165e88ab
author Tomasz Pytel <tompytel@gmail.com> Wed Dec 02 23:32:48 2020 -0300
committer GitHub <noreply@github.com> Thu Dec 03 10:32:48 2020 +0800
tree 172432deba824de0c7b50eb78a0a718cf7737816
parent aae33f532ecd63dc1acee776b1008909d96e1d20

[Enhancement] tweaks for better error handling (#5)

src/plugins/HttpPlugin.ts

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 @@
               };
         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];
+          request.on('close', stopIfNotStopped);
+          request.on('abort', () => (span.errored = true, stopIfNotStopped()));
+          request.on('error', (err) => (span.error(err), stopIfNotStopped()));
 
-        }, arguments);
+          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 @@
 
         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));

src/trace/context/ContextManager.ts

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 @@
       span.start();
 
     try {
-      return callback(span, ...args);
+      return callback(...args);
     } catch (e) {
       span.error(e);
       throw e;
@@ -82,7 +82,7 @@
       span.start();
 
     try {
-      return await callback(span, ...args);
+      return await callback(...args);
     } catch (e) {
       span.error(e);
       throw e;
@@ -90,19 +90,6 @@
       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();