Unhandled exception: Bad state: Header already sent

[afominov]

Hello,

sending any response status code in internal error handler caused an error, when request body is not a valid JSON

Unhandled exception:
Bad state: Header already sent
#0      _HttpResponse.statusCode= (dart:_http/http_impl.dart:1190:35)
#1      errorHandler (test.dart:18:7)
#2      Alfred._incomingRequest (package:alfred/src/alfred.dart:419:35)
<asynchronous suspension>
#3      _QueuedFuture.execute (package:queue/src/dart_queue_base.dart:26:16)
<asynchronous suspension>

Code to reproduce

import 'dart:async';
import 'package:alfred/alfred.dart';

void main() async {
  final app = Alfred(onInternalError: errorHandler);

  app.get('/example', (req, res) {
    final body = req.bodyAsJsonMap;
    res.statusCode = 200;
    return body;
  });

  await app.listen();
}

FutureOr errorHandler(HttpRequest req, HttpResponse res) {
  res.statusCode = 500;
  return {'message': 'error not handled'};
}

CURL

curl --request GET \
  --url http://localhost:3000/example \
  --header 'Content-Type: application/json' \
  --data '{
	"test": true,
}'

If I get it right, the problem is that body parser already send 400 HTTP code https://github.com/rknell/alfred/blob/master/lib/src/body_parser/http_body.dart#L123 Is it intended behaviour?

[rknell]

First of in regards to your question "is it the intended behaviour?" well its expected but I agree could be done better.

From dart:io source code:

  /// The status code of the response.
  ///
  /// Any integer value is accepted. For
  /// the official HTTP status codes use the fields from
  /// [HttpStatus]. If no status code is explicitly set the default
  /// value [HttpStatus.ok] is used.
  ///
  /// The status code must be set before the body is written
  /// to. Setting the status code after writing to the response body or
  /// closing the response will throw a `StateError`.

Ideally I would like to see Alfred alert the user that they can't set the status code twice, but definitely don't want to crash the server.

I'll have a look right now at any internal code and see if I can find a way to detect that the body has been written to and at least Alfred won't be the cause of of any crashes

[rknell]

Ahh just had a dig and the last test I wrote was to handle this scenario:

test(
      'it handles a failed body parser wrapped in a try catch block with a manual return (setting the header twice)',
      () async {
    app.post('/test', (req, res) async {
      try {
        await req.body;
      } catch (e) {
        res.statusCode = 500;
        return {'error': true};
      }
    });
    final response = await http.post(Uri.parse('http://localhost:$port/test'),
        body: '{ "email": "[email protected]",}',
        headers: {'Content-Type': 'application/json'});
    expect(response.statusCode, 400);
  });

So yeah, this is the expected behaviour for now. Open to any improvements.

[afominov]

Thanks for the reply

But in my example, the server crashes when trying to set the 500 HTTP code in the internal error handler.

Did I get right that I just have to manually catch the exception like this?

FutureOr errorHandler(HttpRequest req, HttpResponse res) {
  try {
    res.statusCode = 500;
    return {'message': 'error not handled'};
  } catch (_) {
    // empty catch
  }
}

[seceba]

I recreated your problem, as you said, when we add something to the onInternalError response like ( statuscode header etc), an error occurs and the application crashes.

The fastest way to fix this is to prevent it from crashing with try catch.

All in all a bad experience for someone who decided to tackle this after reading the documentation. Because there is an example given in the documentation without try catch.

Also, although I am not sure, I think the problem is the final dynamic result = final dynamic result on line 418 in the Alfred.dart > line 418: await onInternalError!(request, request.response);

I think it is from there.

Because when I changed it to final dynamic result = "5"; I did not experience any crash, I think it is a situation related to this place.

[rknell]

Hey @seceba,

Can you please point me to the area in the documentation thats incorrect?

[seceba]

Hel @rknell

In the error handling episode. If someone use this example like copy/paste an error occurs and the app crashes.

[rknell]

Hey sorry, I completely forgot about this issue.

I've just written up some new code in v1.0 that fixes it - we had a test that I thought was reproducing the second header but it turns out it wasn't hitting it correctly. The new one closes the connection when a second header is sent and the server doesn't crash. Not 100% that it fixes your exact use case but it does handle it correctly and respond with the first header / statuscode plus puts a message in the console.