Merge pull request #1668 from nmchaves/fix-reporting-circ-reference

Catch Error if JSON.Stringify Fails for Engine Trace
apollographql · Sep 14, 2018 · 91afcc7 · 91afcc7
2 parents 4166098 + c37f191
commit 91afcc7
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 9 deletions.
diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts
@@ -214,11 +214,11 @@ export class EngineReportingAgent<TContext = any> {
         message.byteOffset,
         message.byteLength,
       );
-      gzip(messageBuffer, (err, compressed) => {
+      gzip(messageBuffer, (err, gzipResult) => {
         if (err) {
           reject(err);
         } else {
-          resolve(compressed);
+          resolve(gzipResult);
         }
       });
     });
@@ -232,7 +232,7 @@ export class EngineReportingAgent<TContext = any> {
       // Retry on network errors and 5xx HTTP
       // responses.
       async () => {
-        const response = await fetch(endpointUrl, {
+        const curResponse = await fetch(endpointUrl, {
           method: 'POST',
           headers: {
             'user-agent': 'apollo-engine-reporting',
@@ -242,10 +242,10 @@ export class EngineReportingAgent<TContext = any> {
           body: compressed,
         });
 
-        if (response.status >= 500 && response.status < 600) {
-          throw new Error(`${response.status}: ${response.statusText}`);
+        if (curResponse.status >= 500 && curResponse.status < 600) {
+          throw new Error(`${curResponse.status}: ${curResponse.statusText}`);
         } else {
-          return response;
+          return curResponse;
         }
       },
       {

diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts
@@ -138,9 +138,18 @@ export class EngineReportingExtension<TContext = any>
           // will be sent as '""'.
           this.trace.details!.variablesJson![name] = '';
         } else {
-          this.trace.details!.variablesJson![name] = JSON.stringify(
-            o.variables![name],
-          );
+          try {
+            this.trace.details!.variablesJson![name] = JSON.stringify(
+              o.variables![name],
+            );
+          } catch (e) {
+            // This probably means that the value contains a circular reference,
+            // causing `JSON.stringify()` to throw a TypeError:
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#Issue_with_JSON.stringify()_when_serializing_circular_references
+            this.trace.details!.variablesJson![name] = JSON.stringify(
+              '[Unable to convert value to JSON]',
+            );
+          }
         }
       });
     }