Add the attachment with the content of the file being analyzed.

Change-Id: I75b1c61e2d04cd5e03ef4375e97ef3c9f4add874
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/138384
Commit-Queue: Konstantin Shcheglov <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: scheglov

pkg/analysis_server/lib/src/analysis_server.dart

@@ -39,6 +39,7 @@ import 'package:analysis_server/src/plugin/plugin_manager.dart';
 import 'package:analysis_server/src/plugin/plugin_watcher.dart';
 import 'package:analysis_server/src/protocol_server.dart' as server;
 import 'package:analysis_server/src/search/search_domain.dart';
+import 'package:analysis_server/src/server/crash_reporting_attachments.dart';
 import 'package:analysis_server/src/server/detachable_filesystem_manager.dart';
 import 'package:analysis_server/src/server/diagnostic_server.dart';
 import 'package:analysis_server/src/server/error_notifier.dart';
@@ -167,11 +168,13 @@ class AnalysisServer extends AbstractAnalysisServer {
     ResourceProvider baseResourceProvider,
     AnalysisServerOptions options,
     this.sdkManager,
+    CrashReportingAttachmentsBuilder crashReportingAttachmentsBuilder,
     this.instrumentationService, {
     this.requestStatistics,
     DiagnosticServer diagnosticServer,
     this.detachableFileSystemManager,
-  }) : super(options, diagnosticServer, baseResourceProvider) {
+  }) : super(options, diagnosticServer, crashReportingAttachmentsBuilder,
+            baseResourceProvider) {
     notificationManager = NotificationManager(channel, resourceProvider);
 
     pluginManager = PluginManager(
@@ -866,15 +869,7 @@ class ServerContextManagerCallbacks extends ContextManagerCallbacks {
         // TODO(scheglov) Implement notifications for AnalysisService.IMPLEMENTED.
       }
     });
-    analysisDriver.exceptions.listen((nd.ExceptionResult result) {
-      String message = 'Analysis failed: ${result.path}';
-      if (result.contextKey != null) {
-        message += ' context: ${result.contextKey}';
-      }
-      // TODO(39284): should this exception be silent?
-      AnalysisEngine.instance.instrumentationService.logException(
-          SilentException.wrapInMessage(message, result.exception));
-    });
+    analysisDriver.exceptions.listen(analysisServer.logExceptionResult);
     analysisServer.driverMap[folder] = analysisDriver;
     return analysisDriver;
   }

pkg/analysis_server/lib/src/analysis_server_abstract.dart

@@ -9,6 +9,7 @@ import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/collections.dart';
 import 'package:analysis_server/src/context_manager.dart';
 import 'package:analysis_server/src/domains/completion/available_suggestions.dart';
+import 'package:analysis_server/src/server/crash_reporting_attachments.dart';
 import 'package:analysis_server/src/server/diagnostic_server.dart';
 import 'package:analysis_server/src/services/correction/namespace.dart';
 import 'package:analysis_server/src/services/search/element_visitors.dart';
@@ -39,6 +40,9 @@ abstract class AbstractAnalysisServer {
   /// The options of this server instance.
   AnalysisServerOptions options;
 
+  /// The builder for attachments that should be included into crash reports.
+  final CrashReportingAttachmentsBuilder crashReportingAttachmentsBuilder;
+
   /// The [ContextManager] that handles the mapping from analysis roots to
   /// context directories.
   ContextManager contextManager;
@@ -93,7 +97,10 @@ abstract class AbstractAnalysisServer {
   /// list is lazily created and should be accessed using [analyzedFilesGlobs].
   List<Glob> _analyzedFilesGlobs;
 
-  AbstractAnalysisServer(this.options, this.diagnosticServer,
+  AbstractAnalysisServer(
+      this.options,
+      this.diagnosticServer,
+      this.crashReportingAttachmentsBuilder,
       ResourceProvider baseResourceProvider)
       : resourceProvider = OverlayResourceProvider(baseResourceProvider) {
     performance = performanceDuringStartup;
@@ -287,6 +294,23 @@ abstract class AbstractAnalysisServer {
     });
   }
 
+  void logExceptionResult(nd.ExceptionResult result) {
+    String message = 'Analysis failed: ${result.filePath}';
+    if (result.contextKey != null) {
+      message += ' context: ${result.contextKey}';
+    }
+
+    var attachments =
+        crashReportingAttachmentsBuilder.forExceptionResult(result);
+
+    // TODO(39284): should this exception be silent?
+    AnalysisEngine.instance.instrumentationService.logException(
+      SilentException.wrapInMessage(message, result.exception),
+      null,
+      attachments,
+    );
+  }
+
   /// Notify the declarations tracker that the file with the given [path] was
   /// changed - added, updated, or removed.  Schedule processing of the file.
   void notifyDeclarationsTracker(String path) {

pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart

@@ -27,6 +27,7 @@ import 'package:analysis_server/src/lsp/handlers/handlers.dart';
 import 'package:analysis_server/src/lsp/mapping.dart';
 import 'package:analysis_server/src/plugin/notification_manager.dart';
 import 'package:analysis_server/src/protocol_server.dart' as protocol;
+import 'package:analysis_server/src/server/crash_reporting_attachments.dart';
 import 'package:analysis_server/src/server/diagnostic_server.dart';
 import 'package:analysis_server/src/services/completion/completion_performance.dart'
     show CompletionPerformance;
@@ -118,9 +119,11 @@ class LspAnalysisServer extends AbstractAnalysisServer {
     ResourceProvider baseResourceProvider,
     AnalysisServerOptions options,
     this.sdkManager,
+    CrashReportingAttachmentsBuilder crashReportingAttachmentsBuilder,
     this.instrumentationService, {
     DiagnosticServer diagnosticServer,
-  }) : super(options, diagnosticServer, baseResourceProvider) {
+  }) : super(options, diagnosticServer, crashReportingAttachmentsBuilder,
+            baseResourceProvider) {
     messageHandler = UninitializedStateMessageHandler(this);
     // TODO(dantup): This code is almost identical to AnalysisServer, consider
     // moving it the base class that already holds many of these fields.
@@ -629,15 +632,7 @@ class LspServerContextManagerCallbacks extends ContextManagerCallbacks {
         }
       }
     });
-    analysisDriver.exceptions.listen((nd.ExceptionResult result) {
-      String message = 'Analysis failed: ${result.path}';
-      if (result.contextKey != null) {
-        message += ' context: ${result.contextKey}';
-      }
-      // TODO(39284): should this exception be silent?
-      AnalysisEngine.instance.instrumentationService.logException(
-          SilentException.wrapInMessage(message, result.exception));
-    });
+    analysisDriver.exceptions.listen(analysisServer.logExceptionResult);
     analysisServer.driverMap[folder] = analysisDriver;
     return analysisDriver;
   }

pkg/analysis_server/lib/src/lsp/lsp_socket_server.dart

@@ -8,6 +8,7 @@ import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/lsp/channel/lsp_channel.dart';
 import 'package:analysis_server/src/lsp/constants.dart';
 import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
+import 'package:analysis_server/src/server/crash_reporting_attachments.dart';
 import 'package:analysis_server/src/server/diagnostic_server.dart';
 import 'package:analysis_server/src/socket_server.dart';
 import 'package:analyzer/file_system/physical_file_system.dart';
@@ -75,8 +76,13 @@ class LspSocketServer implements AbstractSocketServer {
           'File read mode was set to the unknown mode: $analysisServerOptions.fileReadMode');
     }
 
-    analysisServer = LspAnalysisServer(serverChannel, resourceProvider,
-        analysisServerOptions, sdkManager, instrumentationService,
+    analysisServer = LspAnalysisServer(
+        serverChannel,
+        resourceProvider,
+        analysisServerOptions,
+        sdkManager,
+        CrashReportingAttachmentsBuilder.empty,
+        instrumentationService,
         diagnosticServer: diagnosticServer);
   }
 }

pkg/analysis_server/lib/src/server/crash_reporting.dart

@@ -5,6 +5,7 @@
 import 'package:analyzer/exception/exception.dart';
 import 'package:analyzer/instrumentation/noop_service.dart';
 import 'package:analyzer/instrumentation/plugin_data.dart';
+import 'package:analyzer/instrumentation/service.dart';
 import 'package:telemetry/crash_reporting.dart';
 
 class CrashReportingInstrumentation extends NoopInstrumentationService {
@@ -13,19 +14,30 @@ class CrashReportingInstrumentation extends NoopInstrumentationService {
   CrashReportingInstrumentation(this.reporter);
 
   @override
-  void logException(dynamic exception, [StackTrace stackTrace]) {
+  void logException(dynamic exception,
+      [StackTrace stackTrace,
+      List<InstrumentationServiceAttachment> attachments]) {
+    var crashReportAttachments = (attachments ?? []).map((e) {
+      return CrashReportAttachment.string(
+        field: 'attachment_${e.id}',
+        value: e.stringValue,
+      );
+    }).toList();
+
     if (exception is CaughtException) {
       // Get the root CaughtException, which matters most for debugging.
       CaughtException root = exception.rootCaughtException;
 
       reporter
-          .sendReport(root.exception, root.stackTrace, comment: root.message)
+          .sendReport(root.exception, root.stackTrace,
+              attachments: crashReportAttachments, comment: root.message)
           .catchError((error) {
         // We silently ignore errors sending crash reports (network issues, ...).
       });
     } else {
       reporter
-          .sendReport(exception, stackTrace ?? StackTrace.current)
+          .sendReport(exception, stackTrace ?? StackTrace.current,
+              attachments: crashReportAttachments)
           .catchError((error) {
         // We silently ignore errors sending crash reports (network issues, ...).
       });

pkg/analysis_server/lib/src/server/crash_reporting_attachments.dart

@@ -0,0 +1,19 @@
+// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analyzer/instrumentation/service.dart';
+import 'package:analyzer/src/dart/analysis/driver.dart' show ExceptionResult;
+
+/// A builder for attachments to include into crash reports.
+class CrashReportingAttachmentsBuilder {
+  static final CrashReportingAttachmentsBuilder empty =
+      CrashReportingAttachmentsBuilder();
+
+  /// Return attachments with information about the analysis exception.
+  List<InstrumentationServiceAttachment> forExceptionResult(
+    ExceptionResult result,
+  ) {
+    return const [];
+  }
+}

pkg/analysis_server/lib/src/server/driver.dart

@@ -12,6 +12,7 @@ import 'package:analysis_server/protocol/protocol_constants.dart'
 import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/lsp/lsp_socket_server.dart';
 import 'package:analysis_server/src/server/crash_reporting.dart';
+import 'package:analysis_server/src/server/crash_reporting_attachments.dart';
 import 'package:analysis_server/src/server/detachable_filesystem_manager.dart';
 import 'package:analysis_server/src/server/dev_server.dart';
 import 'package:analysis_server/src/server/diagnostic_server.dart';
@@ -287,14 +288,17 @@ class Driver implements ServerStarter {
   /// should be used to compute relevance scores.
   static const String USE_NEW_RELEVANCE = 'use-new-relevance';
 
-  /// The instrumentation service that is to be used by the analysis server.
-  InstrumentationService instrumentationService;
+  /// The builder for attachments that should be included into crash reports.
+  CrashReportingAttachmentsBuilder crashReportingAttachmentsBuilder =
+      CrashReportingAttachmentsBuilder.empty;
 
-  /// *
   /// An optional manager to handle file systems which may not always be
   /// available.
   DetachableFileSystemManager detachableFileSystemManager;
 
+  /// The instrumentation service that is to be used by the analysis server.
+  InstrumentationService instrumentationService;
+
   HttpAnalysisServer httpServer;
 
   Driver();
@@ -455,6 +459,7 @@ class Driver implements ServerStarter {
           analysisServerOptions,
           parser,
           dartSdkManager,
+          crashReportingAttachmentsBuilder,
           instrumentationService,
           RequestStatisticsHelper(),
           analytics,
@@ -468,6 +473,7 @@ class Driver implements ServerStarter {
     AnalysisServerOptions analysisServerOptions,
     CommandLineParser parser,
     DartSdkManager dartSdkManager,
+    CrashReportingAttachmentsBuilder crashReportingAttachmentsBuilder,
     InstrumentationService instrumentationService,
     RequestStatisticsHelper requestStatistics,
     telemetry.Analytics analytics,
@@ -500,6 +506,7 @@ class Driver implements ServerStarter {
     final socketServer = SocketServer(
         analysisServerOptions,
         dartSdkManager,
+        crashReportingAttachmentsBuilder,
         instrumentationService,
         requestStatistics,
         diagnosticServer,

pkg/analysis_server/lib/src/server/error_notifier.dart

@@ -8,7 +8,9 @@ class ErrorNotifier extends NoopInstrumentationService {
   AbstractAnalysisServer server;
 
   @override
-  void logException(dynamic exception, [StackTrace stackTrace]) {
+  void logException(dynamic exception,
+      [StackTrace stackTrace,
+      List<InstrumentationServiceAttachment> attachments]) {
     if (exception is SilentException) {
       // Silent exceptions should not be reported to the user.
       return;

pkg/analysis_server/lib/src/socket_server.dart

@@ -7,6 +7,7 @@ import 'package:analysis_server/protocol/protocol_generated.dart';
 import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/analysis_server_abstract.dart';
 import 'package:analysis_server/src/channel/channel.dart';
+import 'package:analysis_server/src/server/crash_reporting_attachments.dart';
 import 'package:analysis_server/src/server/detachable_filesystem_manager.dart';
 import 'package:analysis_server/src/server/diagnostic_server.dart';
 import 'package:analysis_server/src/utilities/request_statistics.dart';
@@ -31,6 +32,7 @@ class SocketServer implements AbstractSocketServer {
   /// The function used to create a new SDK using the default SDK.
   final DartSdkManager sdkManager;
 
+  final CrashReportingAttachmentsBuilder crashReportingAttachmentsBuilder;
   final InstrumentationService instrumentationService;
   final RequestStatisticsHelper requestStatistics;
   @override
@@ -45,6 +47,7 @@ class SocketServer implements AbstractSocketServer {
   SocketServer(
       this.analysisServerOptions,
       this.sdkManager,
+      this.crashReportingAttachmentsBuilder,
       this.instrumentationService,
       this.requestStatistics,
       this.diagnosticServer,
@@ -81,6 +84,7 @@ class SocketServer implements AbstractSocketServer {
       resourceProvider,
       analysisServerOptions,
       sdkManager,
+      crashReportingAttachmentsBuilder,
       instrumentationService,
       requestStatistics: requestStatistics,
       diagnosticServer: diagnosticServer,

pkg/analysis_server/lib/starter.dart

@@ -2,6 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'package:analysis_server/src/server/crash_reporting_attachments.dart';
 import 'package:analysis_server/src/server/detachable_filesystem_manager.dart';
 import 'package:analysis_server/src/server/driver.dart';
 import 'package:analyzer/instrumentation/instrumentation.dart';
@@ -14,6 +15,11 @@ abstract class ServerStarter {
   /// Initialize a newly created starter to start up an analysis server.
   factory ServerStarter() = Driver;
 
+  /// Set the new builder for attachments that should be included into crash
+  /// reports.
+  set crashReportingAttachmentsBuilder(
+      CrashReportingAttachmentsBuilder builder);
+
   /// An optional manager to handle file systems which may not always be
   /// available.
   set detachableFileSystemManager(DetachableFileSystemManager manager);

pkg/analysis_server/test/analysis_abstract.dart

@@ -10,6 +10,7 @@ import 'package:analysis_server/protocol/protocol_generated.dart'
     hide AnalysisOptions;
 import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/domain_analysis.dart';
+import 'package:analysis_server/src/server/crash_reporting_attachments.dart';
 import 'package:analysis_server/src/utilities/mocks.dart';
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/instrumentation/instrumentation.dart';
@@ -125,6 +126,7 @@ class AbstractAnalysisTest with ResourceProviderMixin {
         resourceProvider,
         options,
         DartSdkManager(resourceProvider.convertPath('/sdk'), true),
+        CrashReportingAttachmentsBuilder.empty,
         InstrumentationService.NULL_SERVICE);
   }
 

pkg/analysis_server/test/analysis_server_test.dart

@@ -9,6 +9,7 @@ import 'package:analysis_server/protocol/protocol_constants.dart';
 import 'package:analysis_server/protocol/protocol_generated.dart';
 import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/domain_server.dart';
+import 'package:analysis_server/src/server/crash_reporting_attachments.dart';
 import 'package:analysis_server/src/utilities/mocks.dart';
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/instrumentation/instrumentation.dart';
@@ -86,6 +87,7 @@ class AnalysisServerTest with ResourceProviderMixin {
         resourceProvider,
         AnalysisServerOptions(),
         DartSdkManager(convertPath('/sdk'), false),
+        CrashReportingAttachmentsBuilder.empty,
         InstrumentationService.NULL_SERVICE);
   }
 

pkg/analysis_server/test/client/impl/abstract_client.dart

@@ -10,6 +10,7 @@ import 'package:analysis_server/protocol/protocol_generated.dart'
 import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/domain_analysis.dart';
 import 'package:analysis_server/src/domain_completion.dart';
+import 'package:analysis_server/src/server/crash_reporting_attachments.dart';
 import 'package:analysis_server/src/utilities/mocks.dart';
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/instrumentation/instrumentation.dart';
@@ -104,8 +105,13 @@ abstract class AbstractClient {
   AnalysisServer createAnalysisServer(String sdkPath) {
     sdk = MockSdk(resourceProvider: resourceProvider);
     var options = AnalysisServerOptions();
-    return AnalysisServer(serverChannel, resourceProvider, options,
-        DartSdkManager(sdkPath, true), InstrumentationService.NULL_SERVICE);
+    return AnalysisServer(
+        serverChannel,
+        resourceProvider,
+        options,
+        DartSdkManager(sdkPath, true),
+        CrashReportingAttachmentsBuilder.empty,
+        InstrumentationService.NULL_SERVICE);
   }
 
   /// Create a project at [projectPath].

pkg/analysis_server/test/context_manager_test.dart

@@ -2404,7 +2404,7 @@ class TestContextManagerCallbacks extends ContextManagerCallbacks {
     driverMap[path] = currentDriver;
     currentDriver.exceptions.listen((ExceptionResult result) {
       AnalysisEngine.instance.instrumentationService.logException(
-          CaughtException.withMessage('Analysis failed: ${result.path}',
+          CaughtException.withMessage('Analysis failed: ${result.filePath}',
               result.exception.exception, result.exception.stackTrace));
     });
     return currentDriver;