Support Request hijacking in Shelf, using a similar API to Rack.

Support Request hijacking in Shelf, using a similar API to Rack.

R=kevmoo@google.com

Committed: https://code.google.com/p/dart/source/detail?r=36324

[kevmoo, 2014-05-07 19:11:34]

Would you rebase on latest? I had a merge conflict. Likely since my
change...change

[kevmoo, 2014-05-07 19:28:22]

I'm struggling a bit w/ making folks special-case a particular Exception or
special-case a particular Response type.

We should talk...

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/shelf_io.dart
File pkg/shelf/lib/shelf_io.dart (right):

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/shelf_io.dart#...
pkg/shelf/lib/shelf_io.dart:80: ..writeln("${response.statusCode}");
writeln takes an object. No need to do string interpolation here, right?

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/handlers/l...
File pkg/shelf/lib/src/handlers/logger.dart (right):

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/handlers/l...
pkg/shelf/lib/src/handlers/logger.dart:41: if (error is HijackException) throw
error;
Is this something we want to generalize? A base class for "functional
exceptions".

Having to special case any more of these will be annoying.

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/hijack_exc...
File pkg/shelf/lib/src/hijack_exception.dart (right):

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/hijack_exc...
pkg/shelf/lib/src/hijack_exception.dart:20: "Shelf adapter.";
Un-dent 2 spaces

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/request.dart
File pkg/shelf/lib/src/request.dart (right):

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/request.da...
pkg/shelf/lib/src/request.dart:108: /// indicate that hijacking has occurred;
the adapter should avoid either
occurred. The adapter...

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/request.da...
pkg/shelf/lib/src/request.dart:109: /// sending a response or notifying the user
of an error if one of these is
"one of these is received" -> a [HijackException] is caught.

Just to be super clear.

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/request.da...
pkg/shelf/lib/src/request.dart:193: var called = false;
var -> bool

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/test/hijack_test.dart
File pkg/shelf/test/hijack_test.dart (right):

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/test/hijack_test.d...
pkg/shelf/test/hijack_test.dart:51: }
End with a newline

[nweiz, 2014-05-19 20:10:28]

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/shelf_io.dart
File pkg/shelf/lib/shelf_io.dart (right):

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/shelf_io.dart#...
pkg/shelf/lib/shelf_io.dart:80: ..writeln("${response.statusCode}");
On 2014/05/07 19:28:22, kevmoo wrote:
> writeln takes an object. No need to do string interpolation here, right?

Done.

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/handlers/l...
File pkg/shelf/lib/src/handlers/logger.dart (right):

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/handlers/l...
pkg/shelf/lib/src/handlers/logger.dart:41: if (error is HijackException) throw
error;
On 2014/05/07 19:28:22, kevmoo wrote:
> Is this something we want to generalize? A base class for "functional
> exceptions".
> 
> Having to special case any more of these will be annoying.

As per our in-person discussion, I think we're unlikely to want additional
exceptions that work this way.

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/hijack_exc...
File pkg/shelf/lib/src/hijack_exception.dart (right):

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/hijack_exc...
pkg/shelf/lib/src/hijack_exception.dart:20: "Shelf adapter.";
On 2014/05/07 19:28:22, kevmoo wrote:
> Un-dent 2 spaces

I'll do this if you really want, but when writing multiline strings like this I
like to add a bit of extra indentation on continued lines to visually
distinguish them from new lines.

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/request.dart
File pkg/shelf/lib/src/request.dart (right):

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/request.da...
pkg/shelf/lib/src/request.dart:108: /// indicate that hijacking has occurred;
the adapter should avoid either
On 2014/05/07 19:28:22, kevmoo wrote:
> occurred. The adapter...

Done.

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/request.da...
pkg/shelf/lib/src/request.dart:109: /// sending a response or notifying the user
of an error if one of these is
On 2014/05/07 19:28:22, kevmoo wrote:
> "one of these is received" -> a [HijackException] is caught.
> 
> Just to be super clear.

Done.

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/request.da...
pkg/shelf/lib/src/request.dart:193: var called = false;
On 2014/05/07 19:28:22, kevmoo wrote:
> var -> bool

Done, under protest.

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/test/hijack_test.dart
File pkg/shelf/test/hijack_test.dart (right):

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/test/hijack_test.d...
pkg/shelf/test/hijack_test.dart:51: }
On 2014/05/07 19:28:22, kevmoo wrote:
> End with a newline

Done.

[kevmoo, 2014-05-19 20:14:17]

lgtm w/ comments

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/hijack_exc...
File pkg/shelf/lib/src/hijack_exception.dart (right):

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/hijack_exc...
pkg/shelf/lib/src/hijack_exception.dart:20: "Shelf adapter.";
On 2014/05/19 20:10:29, nweiz wrote:
> On 2014/05/07 19:28:22, kevmoo wrote:
> > Un-dent 2 spaces
> 
> I'll do this if you really want, but when writing multiline strings like this
I
> like to add a bit of extra indentation on continued lines to visually
> distinguish them from new lines.

I see you point. Not a problem.

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/request.dart
File pkg/shelf/lib/src/request.dart (right):

https://codereview.chromium.org/260933004/diff/1/pkg/shelf/lib/src/request.da...
pkg/shelf/lib/src/request.dart:193: var called = false;
On 2014/05/19 20:10:29, nweiz wrote:
> On 2014/05/07 19:28:22, kevmoo wrote:
> > var -> bool
> 
> Done, under protest.

:-)

[nweiz, 2014-05-19 21:00:59]

Committed patchset #2 manually as r36324 (presubmit successful).