Fix error reporting when error handlers fail

dart-bridge · Dec 11, 2015 · 54fc3c2 · 54fc3c2
1 parent 3a609a7
commit 54fc3c2
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 19 deletions.
diff --git a/lib/src/http/exceptions/http_not_found_exception.dart b/lib/src/http/exceptions/http_not_found_exception.dart
@@ -15,5 +15,5 @@ class HttpNotFoundException implements Exception {
       return 'The file at ${path.relative(_file.path)}';
   }
 
-  String toString() => '$resource was not found';
+  String toString() => 'HttpNotFoundException: $resource was not found';
 }
diff --git a/lib/src/http/middleware.dart b/lib/src/http/middleware.dart
@@ -8,13 +8,31 @@ abstract class Middleware {
     return handle;
   }
 
-  Future<shelf.Response> handle(shelf.Request request) async {
+  Future<shelf.Response> handle(shelf.Request request) {
+    return _handle(request).then(_reattach(new PipelineAttachment.of(request)));
+  }
+
+  Future<shelf.Response> _handle(shelf.Request request) async {
     return await _inner(request);
   }
 
+  Function _reattach(PipelineAttachment attachment) {
+    return (shelf.Response response) {
+      return attachIfEmpty(response, attachment);
+    };
+  }
+
   shelf.Message attach(shelf.Message message, PipelineAttachment attachment) {
     return message.change(context: {
-      PipelineAttachment._contextKey: new PipelineAttachment.of(message).apply(attachment)
+      PipelineAttachment._contextKey: new PipelineAttachment.of(message)
+          .apply(attachment)
+    });
+  }
+
+  shelf.Message attachIfEmpty(shelf.Message message, PipelineAttachment attachment) {
+    return message.change(context: {
+      PipelineAttachment._contextKey: new PipelineAttachment.of(message)
+          .applyIfEmpty(attachment)
     });
   }
 
@@ -68,4 +86,20 @@ class PipelineAttachment {
         session: other.session ?? session
     );
   }
+
+  PipelineAttachment applyIfEmpty(PipelineAttachment other) {
+    final oldInject = new Map.from(inject);
+    for (final injectKey in other.inject.keys) {
+      oldInject.putIfAbsent(injectKey, () => other.inject[injectKey]);
+    }
+    final oldConvert = new Map.from(convert);
+    for (final convertKey in other.convert.keys) {
+      oldConvert.putIfAbsent(convertKey, () => other.convert[convertKey]);
+    }
+    return new PipelineAttachment(
+        inject: oldInject,
+        convert: oldConvert,
+        session: session ?? other.session
+    );
+  }
 }
diff --git a/lib/src/http/pipeline.dart b/lib/src/http/pipeline.dart
@@ -168,14 +168,17 @@ class _GlobalErrorHandlerMiddleware extends Middleware {
       return await super.handle(request);
     } on shelf.HijackException {
       rethrow;
-    } catch(e, s) {
+    } catch(_e, _s) {
+      var error = _e;
+      StackTrace stackTrace = _s;
       try {
-        final attempt = await _handlers(request, e, s);
+        final attempt = await _handlers(request, error, stackTrace);
         if (attempt is shelf.Response) return attempt;
-      } catch (_) {
-        return new Future.error(e, s);
+      } catch (_e, _s) {
+        error = _e;
+        stackTrace = _s;
       }
-      final stack = new Chain.forTrace(s).terse
+      final stack = new Chain.forTrace(stackTrace).terse
           .toString()
           .replaceAll(new RegExp(r'.*Program\.execute[^]*'),
           '===== program started ============================\n')
@@ -186,29 +189,29 @@ class _GlobalErrorHandlerMiddleware extends Middleware {
           .split('\n')
           .reversed
           .join('\n');
-      final message = '   ' + e.toString().replaceAll('\n', '\n   ');
+      final message = '   ' + error.toString().replaceAll('\n', '\n   ');
       print('''<yellow>$stack</yellow>
 <red-background><white>
 
 $message
 </white></red-background>
 
-<yellow><bold>Note:</bold> To mute this message, add an error handler for <underline>${e.runtimeType}</underline>:</yellow>
+<yellow><bold>Note:</bold> To mute this message, add an error handler for <underline>${error.runtimeType}</underline>:</yellow>
 
 <yellow>class</yellow> <cyan>Main</cyan> <yellow>extends</yellow> <cyan>Pipeline</cyan> {
   <green>@override</green> <yellow>get</yellow> errorHandlers => {
-    <cyan>${e.runtimeType}</cyan>: _handle${e.runtimeType}
+    <cyan>${error.runtimeType}</cyan>: _handle${error.runtimeType}
   };
 
-  _handle${e.runtimeType}(<cyan>${e.runtimeType}</cyan> error) {
-    <yellow>return</yellow> <red>'Ouch! We encountered a(n) ${e.runtimeType}! Sorry about that!'</red>;
+  _handle${error.runtimeType}(<cyan>${error.runtimeType}</cyan> error) {
+    <yellow>return</yellow> <red>'Ouch! We encountered a(n) ${error.runtimeType}! Sorry about that!'</red>;
   }
 }
 ''');
 
-      if (e is HttpNotFoundException)
-        return new shelf.Response.notFound('$e');
-      return new shelf.Response.internalServerError(body: '$e');
+      if (error is HttpNotFoundException)
+        return new shelf.Response.notFound('$error');
+      return new shelf.Response.internalServerError(body: '$error');
     }
   }
 }
diff --git a/lib/src/http/sessions/sessions_middleware.dart b/lib/src/http/sessions/sessions_middleware.dart
@@ -8,12 +8,12 @@ class SessionsMiddleware extends Middleware {
   Future<shelf.Response> handle(shelf.Request request) async {
     final session = manager.attachSession(request);
     return super.handle(applySession(request, session))
-        .then(_attachSessionToResponse);
+        .then(_treatResponse);
   }
 
-  shelf.Response _attachSessionToResponse(shelf.Response response) {
+  shelf.Response _treatResponse(shelf.Response response) {
     final session = manager.sessionOf(response);
-    if (session?.isNew ?? true) response = _setSessionCookie(response, session);
+    if (session.isNew) response = _setSessionCookie(response, session);
     session.clearOldFlashes();
     return response;
   }