Fixes for forgotten review comments.

pkg/analyzer_experimental/lib/src/services/runtime/coverage/coverage_impl.dart

 // Copyright (c) 2013, 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.
 
 /// A library for code coverage support for Dart.
 library runtime.coverage.impl;
 
 import 'dart:async';
 import 'dart:collection' show SplayTreeMap;
 import 'dart:io';
@@ skipping 10 matching lines @@
 import '../log.dart' as log;
 import 'models.dart';
 import 'utils.dart';
 
 /// Run the [targetPath] with code coverage rewriting.
 /// Redirects stdandard process streams.
 /// On process exit dumps coverage statistics into the [outPath].
 void runServerApplication(String targetPath, String outPath) {
   var targetFolder = pathos.dirname(targetPath);
   var targetName = pathos.basename(targetPath);
-  new CoverageServer(targetFolder, targetPath, outPath)
-      .start()
-      .then((port) {
-        var options = new Options();
-        var targetArgs = ['http://127.0.0.1:$port/$targetName'];
-        var dartExecutable = options.executable;
-        Process.start(dartExecutable, targetArgs).then((Process process) {
-          process.exitCode.then(exit);
-          // Redirect process streams.
-          stdin.pipe(process.stdin);
-          process.stdout.pipe(stdout);
-          process.stderr.pipe(stderr);
-        });
-      });
+  var server = new CoverageServer(targetFolder, targetPath, outPath);
+  server.start().then((port) {
+    var options = new Options();
+    var targetArgs = ['http://127.0.0.1:$port/$targetName'];
+    var dartExecutable = options.executable;
+    return Process.start(dartExecutable, targetArgs);
+  }).then((process) {
+    stdin.pipe(process.stdin);
+    process.stdout.pipe(stdout);
+    process.stderr.pipe(stderr);
+    return process.exitCode;
+  }).then(exit).catchError((e) {
+    log.severe('Error starting $targetPath. $e');
+  });
 }
 
 
 /// Abstract server to listen requests and serve files, may be rewriting them.
 abstract class RewriteServer {
   final String basePath;
   int port;
 
   RewriteServer(this.basePath);
 
   /// Runs the HTTP server on the ephemeral port and returns [Future] with it.
   Future<int> start() {
     return HttpServer.bind('127.0.0.1', 0).then((server) {
       port = server.port;
       log.info('RewriteServer is listening at: $port.');
       server.listen((request) {
         if (request.method == 'GET') {
           handleGetRequest(request);
         }
         if (request.method == 'POST') {
           handlePostRequest(request);
         }
       });
       return port;
     });
   }
 
-  handlePostRequest(HttpRequest request);
+  void handlePostRequest(HttpRequest request);
 
-  handleGetRequest(HttpRequest request) {
+  void handleGetRequest(HttpRequest request) {
     var response = request.response;
     // Prepare path.
-    var path = basePath + '/' + request.uri.path;
-    path = pathos.normalize(path);
+    var path = getFilePath(request.uri);
     log.info('[$path] Requested.');
     // May be serve using just path.
     {
       var content = rewritePathContent(path);
       if (content != null) {
         log.info('[$path] Request served by path.');
         response.write(content);
         response.close();
         return;
       }
     }
     // Serve from file.
     log.info('[$path] Serving file.');
     var file = new File(path);
     file.exists().then((found) {
       if (found) {
         // May be this files should be sent as is.
         if (!shouldRewriteFile(path)) {
-          sendFile(request, file);
-          return;
+          return sendFile(request, file);
         }
         // Rewrite content of the file.
-        file.readAsString().then((content) {
+        return file.readAsString().then((content) {
           log.finest('[$path] Done reading ${content.length} characters.');
           content = rewriteFileContent(path, content);
           log.fine('[$path] Rewritten.');
           response.write(content);
-          response.close();
+          return response.close();
         });
       } else {
         log.severe('[$path] File not found.');
         response.statusCode = HttpStatus.NOT_FOUND;
-        response.close();
+        return response.close();
       }
+    }).catchError((e) {
+      log.severe('[$path] $e.');
+      response.statusCode = HttpStatus.INTERNAL_SERVER_ERROR;
+      return response.close();
     });
   }
 
-  void sendFile(HttpRequest request, File file) {
+  String getFilePath(Uri uri) {
+    var path = uri.path;
+    path = pathos.joinAll(uri.pathSegments);
+    path = pathos.join(basePath, path);
+    return pathos.normalize(path);
+  }
+
+  Future sendFile(HttpRequest request, File file) {
     file.fullPath().then((fullPath) {
-      file.openRead()
-        .pipe(request.response)
-        .catchError((e) {});
+      return file.openRead().pipe(request.response);
     });
   }
 
   bool shouldRewriteFile(String path);
 
   /// Subclasses implement this method to rewrite the provided [code] of the
   /// file with [path]. Returns some content or `null` if file content
   /// should be requested.
   String rewritePathContent(String path);
 
   /// Subclasses implement this method to rewrite the provided [code] of the
   /// file with [path].
   String rewriteFileContent(String path, String code);
 }
 
 
+/// Here `CCC` means 'code coverage configuration'.
+const TEST_UNIT_CCC = '''
+class __CCC extends __cc_ut.Configuration {
+  void onDone(bool success) {
+    __cc.postStatistics();
+    super.onDone(success);
+  }
+}''';
+
+const TEST_UNIT_CCC_SET = '__cc_ut.unittestConfiguration = new __CCC();';
+
+
 /// Server that rewrites Dart code so that it reports execution of statements
 /// and other nodes.
 class CoverageServer extends RewriteServer {
   final appInfo = new AppInfo();
   final String targetPath;
   final String outPath;
 
   CoverageServer(String basePath, this.targetPath, this.outPath)
       : super(basePath);
 
   void handlePostRequest(HttpRequest request) {
     var id = 0;
     var executedIds = new Set<int>();
     request.listen((data) {
       log.fine('Received statistics, ${data.length} bytes.');
       while (true) {
         var listIndex = id ~/ 8;
         if (listIndex >= data.length) break;
         var bitIndex = id % 8;
         if ((data[listIndex] & (1 << bitIndex)) != 0) {
           executedIds.add(id);
         }
         id++;
       }
     }).onDone(() {
       log.fine('Received all statistics.');
-      var sb = new StringBuffer();
-      appInfo.write(sb, executedIds);
-      new File(outPath).writeAsString(sb.toString());
-      log.fine('Results are written to $outPath.');
-      request.response.close();
+      var buffer = new StringBuffer();
+      appInfo.write(buffer, executedIds);
+      new File(outPath).writeAsString(buffer.toString()).then((_) {
+        return request.response.close();
+      }).catchError((e) {
+        log.severe('Error in receiving statistics $e.');
+        return request.response.close();
+      });
     });
   }
 
   String rewritePathContent(String path) {
     if (path.endsWith('__coverage_lib.dart')) {
       String implPath = pathos.joinAll([
           pathos.dirname(new Options().script),
           '..', 'lib', 'src', 'services', 'runtime', 'coverage',
           'coverage_lib.dart']);
       var content = new File(implPath).readAsStringSync();
-      content = content.replaceAll('0; // replaced during rewrite', '$port;');
-      return content;
+      return content.replaceAll('0; // replaced during rewrite', '$port;');
     }
     return null;
   }
 
   bool shouldRewriteFile(String path) {
     if (pathos.extension(path).toLowerCase() != '.dart') return false;
     // Rewrite target itself, only to send statistics.
     if (path == targetPath) {
       return true;
     }
     // TODO(scheglov) use configuration
-    if (path.contains('/packages/analyzer_experimental/')) {
-      return true;
-    }
-    return false;
+    return path.contains('/packages/analyzer_experimental/');
   }
 
   String rewriteFileContent(String path, String code) {
     var unit = _parseCode(code);
     log.finest('[$path] Parsed.');
     var injector = new CodeInjector(code);
     // Inject imports.
     var directives = unit.directives;
     if (directives.isNotEmpty && directives[0] is LibraryDirective) {
       injector.inject(directives[0].end,
           'import "package:unittest/unittest.dart" as __cc_ut;'
           'import "http://127.0.0.1:$port/__coverage_lib.dart" as __cc;');
     }
     // Inject statistics sender.
     var isTargetScript = path == targetPath;
     if (isTargetScript) {
       for (var node in unit.declarations) {
         if (node is FunctionDeclaration) {
           var body = node.functionExpression.body;
           if (node.name.name == 'main' && body is BlockFunctionBody) {
-            injector.inject(node.offset,
-                'class __CCC extends __cc_ut.Configuration {'
-                '  void onDone(bool success) {'
-                '    __cc.postStatistics();'
-                '    super.onDone(success);'
-                '  }'
-                '}');
-            injector.inject(
-                body.offset + 1,
-                '__cc_ut.unittestConfiguration = new __CCC();');
+            injector.inject(node.offset, TEST_UNIT_CCC);
+            injector.inject(body.offset + 1, TEST_UNIT_CCC_SET);
           }
         }
       }
     }
     // Inject touch() invocations.
     if (!isTargetScript) {
       appInfo.enterUnit(path, code);
       unit.accept(new InsertTouchInvocationsVisitor(appInfo, injector));
     }
     // Done.
@@ skipping 88 matching lines @@
     var lastOffset = 0;
     offsetFragmentMap.forEach((offset, fragment) {
       sb.write(_code.substring(lastOffset, offset));
       sb.write(fragment);
       lastOffset = offset;
     });
     sb.write(_code.substring(lastOffset, _code.length));
     return sb.toString();
   }
 }