Specify whether error serialization timing should be captured

A previous commit
#2900 migrated the
extension for reporting tracing information to engine into an
abstraction called the treeBuilder. The treeBuilder was then utilized by
both a federatedExtension and the normal extension, but the two
extensions have different end-timing semantics. In the normal extension,
the end-timing is captured at the end of the *request*, whereas in the
federated extension, the end-timing is captured at the end of the
*execution*. The reason for this is that the standard exception does not
actually append information into the extensions field, whereas the
federated extension does, so serialization and transmission of the
response cannot be included in the end of the extension.

Due to the limitations of the current requestPipeline, executionDidEnd
is what needs to be hooked into to halt overall timing, which is called
directly prior to didEncounterErrors. It is likely possible to refactor
the request pipeline to support capturing the timing data of serializing
errors into the proto state, but since it would be more boolean
control-flow to add into the overall request context, this commit simply
discards that timing information for reporting in lieu of throwing an
error.

A subsequent commit will follow up with replacing error throws with
logging statements.

packages/apollo-engine-reporting/src/extension.ts

@@ -196,7 +196,7 @@ export class EngineReportingExtension<TContext = any>
   }
 
   public didEncounterErrors(errors: GraphQLError[]) {
-    this.treeBuilder.didEncounterErrors(errors);
+    this.treeBuilder.didEncounterErrors(errors, true);
   }
 }
 

packages/apollo-engine-reporting/src/federatedExtension.ts

@@ -49,7 +49,7 @@ export class EngineFederatedTracingExtension<TContext = any>
 
   public didEncounterErrors(errors: GraphQLError[]) {
     if (this.enabled) {
-      this.treeBuilder.didEncounterErrors(errors);
+      this.treeBuilder.didEncounterErrors(errors, false);
     }
   }
 

packages/apollo-engine-reporting/src/treeBuilder.ts

@@ -71,7 +71,10 @@ export class EngineReportingTreeBuilder {
     };
   }
 
-  public didEncounterErrors(errors: GraphQLError[]) {
+  public didEncounterErrors(
+    errors: GraphQLError[],
+    shouldIncludeErrorTiming: boolean,
+  ) {
     errors.forEach(err => {
       // This is an error from a federated service. We will already be reporting
       // it in the nested Trace in the query plan.
@@ -92,19 +95,29 @@ export class EngineReportingTreeBuilder {
       this.addProtobufError(
         errorForReporting.path,
         errorToProtobufError(errorForReporting),
+        shouldIncludeErrorTiming,
       );
     });
   }
 
   private addProtobufError(
     path: ReadonlyArray<string | number> | undefined,
     error: Trace.Error,
+    shouldIncludeErrorTiming: boolean,
   ) {
     if (!this.startHrTime) {
       throw Error('addProtobufError called before startTiming!');
     }
-    if (this.stopped) {
-      throw Error('addProtobufError called after stopTiming!');
+    if (this.stopped && shouldIncludeErrorTiming) {
+      throw Error(
+        'addProtobufError called after stopTiming ' +
+          'when we should include error serialization timing',
+      );
+    } else if (!this.stopped && !shouldIncludeErrorTiming) {
+      throw Error(
+        'addProtobufError called before stopTiming ' +
+          'when we should not include error serialization timing',
+      );
     }
 
     // By default, put errors on the root node.