pkg/http_server: return future for VirtualDirectory serveRequest

pkg/http_server/lib/src/virtual_directory.dart

Index: pkg/http_server/lib/src/virtual_directory.dart
diff --git a/pkg/http_server/lib/src/virtual_directory.dart b/pkg/http_server/lib/src/virtual_directory.dart
index 7665a71e7cb40dec5f1b6793a54a273e9af19c97..2d5e5e6e97bf8d2ea31256c4fc6c1b22986ebbc8 100644
--- a/pkg/http_server/lib/src/virtual_directory.dart
+++ b/pkg/http_server/lib/src/virtual_directory.dart
@@ -4,6 +4,9 @@
 
 part of http_server;
 
+typedef dynamic _DirCallback(Directory dir, HttpRequest request);
+typedef dynamic _ErrorCallback(HttpRequest request);
+
 /**
  * A [VirtualDirectory] can serve files and directory-listing from a root path,
  * to [HttpRequest]s.
@@ -34,8 +37,8 @@ class VirtualDirectory {
 
   final RegExp _invalidPathRegExp = new RegExp("[\\\/\x00]");
 
-  Function _errorCallback;
-  Function _dirCallback;
+  _ErrorCallback _errorCallback;
+  _DirCallback _dirCallback;
 
   /*
    * Create a new [VirtualDirectory] for serving static file content of
@@ -49,20 +52,15 @@ class VirtualDirectory {
   /**
    * Serve a [Stream] of [HttpRequest]s, in this [VirtualDirectory].
    */
-  void serve(Stream<HttpRequest> requests) {
-    requests.listen(serveRequest);
-  }
+  StreamSubscription<HttpRequest> serve(Stream<HttpRequest> requests) =>
+      requests.listen(serveRequest);
 
   /**
    * Serve a single [HttpRequest], in this [VirtualDirectory].
    */
-  void serveRequest(HttpRequest request) {
-    _locateResource('.', request.uri.pathSegments.iterator..moveNext())
-        .then((entity) {
-          if (entity == null) {
-            _serveErrorPage(HttpStatus.NOT_FOUND, request);
-            return;
-          }
+  Future serveRequest(HttpRequest request) {
+    return _locateResource('.', request.uri.pathSegments.iterator..moveNext())
+        .then((FileSystemEntity entity) {
           if (entity is File) {
             serveFile(entity, request);
           } else if (entity is Directory) {
@@ -70,6 +68,7 @@ class VirtualDirectory {
           } else {
             _serveErrorPage(HttpStatus.NOT_FOUND, request);
           }
+          return request.response.done;
         });
   }
 
@@ -166,7 +165,7 @@ class VirtualDirectory {
           !lastModified.isAfter(request.headers.ifModifiedSince)) {
         response.statusCode = HttpStatus.NOT_MODIFIED;
         response.close();
-        return;
+        return null;
       }
 
       response.headers.set(HttpHeaders.LAST_MODIFIED, lastModified);
@@ -174,7 +173,7 @@ class VirtualDirectory {
 
       if (request.method == 'HEAD') {
         response.close();
-        return;
+        return null;
       }
 
       return file.length().then((length) {
@@ -283,8 +282,7 @@ $server
         add('../', null, null);
       }
 
-      dir.list(followLinks: true).listen((entity) {
-        // TODO(ajohnsen): Consider async dir listing.
+      dir.list(followLinks: true).forEach((entity) {

[Anders Johnsen, 2014/01/09 06:12:23]

This is not handling errors (they can happen if FS is modified while listing).

[kevmoo, 2014/01/10 18:12:31]

I think I've addressed this, with TODOs for logging errors

         if (entity is File) {
           var stat = entity.statSync();
           add(basename(entity.path),
@@ -295,12 +293,14 @@ $server
               entity.statSync().modified.toString(),
               null);
         }
-      }, onError: (e) {
-      }, onDone: () {
+      })
+      .then((_) {
         response.write(footer);
-        response.close();
+      })
+      .whenComplete(() {

[sethladd, 2014/01/09 05:27:48]

can this be a one-line function?

[kevmoo, 2014/01/10 05:02:00]

Done.

+        return response.close();

[Anders Johnsen, 2014/01/09 06:12:23]

Why return here? This value is not kept.

[kevmoo, 2014/01/10 05:02:00]

Done.

       });
-    }, onError: (e) => response.close());
+    });
   }
 
   void _serveErrorPage(int error, HttpRequest request) {