Error handler based on the request inputs

[beajeanm]

This maybe more a question than a bug/feature request.

The error handler I'm trying to implement has some logic based on the incoming request that generated the error.

If building it from scratch (so implementing Dream.error -> Dream.response option Lwt.t) then it's all good, I have the (optional) request in error.request. But I'm missing all the bells an whistle that come with the default error handler. (Smart logging and stack trace in debug mode.)
If I try to go through the template instead (so implementing string option -> response -> response Lwt.t) I lose access to all the request inputs.

I can see how to workaround the problem for reponse based errors I generated by adding the required data to the reponse locals. But that only cover part of the errors, and it doesn't work for the reponses generated by another middleware (e.g. Dream.not_found)

Is there something I'm missing?
Is there a way for me to access the logging and backtrace printing without using the error template?

[thangngoc89]

@beajeanm I believe this is the error_handler https://aantron.github.io/dream/#type-error_handler .

[beajeanm]

Thank you for your comment, but I know about that type (it's the one I describe in building it from scratch) and I don't see how that documentation address my need: Have the logging and back trace logic without using the template.

[aantron]

@beajeanm If I understood you correctly, you'd like to implement a non-trivial error handler (Dream.error -> Dream.response option Lwt.t), and you'd like to reuse some of the logic that is found in the default error handler:

dream/src/http/error_handler.ml

Lines 116 to 187 in 52d4da2

	let customize template (error : Dream.error) =
	(* First, log the error. *)
	begin match error.condition with
	| `Response _ -> ()
	| `String _ | `Exn _ as condition ->
	let client =
	match error.client with
	| None -> ""
	| Some client -> " (" ^ client ^ ")"
	in
	let layer =
	match error.layer with
	| `TLS -> ["TLS" ^ client]
	| `HTTP -> ["HTTP" ^ client]
	| `HTTP2 -> ["HTTP/2" ^ client]
	| `WebSocket -> ["WebSocket" ^ client]
	| `App -> []
	in
	let description, backtrace =
	match condition with
	| `String string -> string, ""
	| `Exn exn ->
	let backtrace = Printexc.get_backtrace () in
	Printexc.to_string exn, backtrace
	in
	let message = String.concat ": " (layer @ [description]) in
	select_log error.severity (fun log ->
	log ?request:error.request "%s" message);
	backtrace |> Dream__middleware.Log.iter_backtrace (fun line ->
	select_log error.severity (fun log ->
	log ?request:error.request "%s" line))
	end;
	(* If Dream will not send a response for this error, we are done after
	logging. Otherwise, if debugging is enabled, gather a bunch of information.
	Then, call the template, and return the response. *)
	if not error.will_send_response then
	Lwt.return_none
	else
	let debug_dump =
	match error.debug with
	| false -> None
	| true -> Some (dump error)
	in
	let response =
	match error.condition with
	| `Response response -> response
	| _ ->
	let status =
	match error.caused_by with
	| `Server -> `Internal_Server_Error
	| `Client -> `Bad_Request
	in
	Dream.response ~status ""
	in
	(* No need to catch errors when calling the template, because every call
	site of the error handler already has error handlers for catching double
	faults. *)
	response
	|> template debug_dump
	|> Lwt.map (fun response -> Some response)

To proceed, I suggest writing a similar implementation in your own handler. I see at least two kinds of things that we may need to support you with:

• Some of the logic in the default error handler may be useful as exposed helpers, so you can use it as building blocks in your custom handler.
• Some of the existing helpers currently used by the default error handler might be internal (I didn't look through in detail just now). We may need to expose those helpers, so that your error handler can also call them.

There may be other things to do.

I see that you want

(Smart logging and stack trace in debug mode.)

However, I'm not sure what you mean by smart logging. The default error handler just formats the information in the Dream.error object in some way, and logs that. Do you want that exposed as a helper, so you can readily do the same logging without the annoyance of writing out and/or maintaining code for it?

The rest of the logic in the default error handler is about the backtrace. It is ultimately just based on Printexc.get_backtrace, but does a bit of formatting work. Would you like to have a helper that logs the backtrace in exactly the same way as the default error handler?

Could you say exactly which pieces you'd like to reuse?

[beajeanm]

@aantron I'm exploring using Dream with inertiajs.
I'll spare the details, but for the issue at hands, it boils down to: The handler should either give an HTML response or a JSON response based on a header in the incoming request.
Doing that the error handler by creating one from scratch, means I lose the nice debugging functionalities or I have to re-implement them.

To proceed, I suggest writing a similar implementation in your own handler. I see at least two kinds of things that we may need to support you with:
Some of the logic in the default error handler may be useful as exposed helpers, so you can use it as building blocks in your custom handler.
Some of the existing helpers currently used by the default error handler might be internal (I didn't look through in detail just now). We may need to expose those helpers, so that your error handler can also call them.

I had a look at the default handler code and tried to add it to my own:

• Logging: you are correct, it's simpler that I thought. I can replace the use of let log = Dream__middleware.Log.sub_log "dream.http" with the public logging methods and get mostly the same result.
• Backtrace: I haven't found a way to make it work without Dream__pure.Inmost, if that's not possible, then having something similar to

  dream/src/http/error_handler.ml

  Lines 28 to 111 in 52d4da2

  	let dump (error : Dream.error) =
  	let buffer = Buffer.create 4096 in
  	let p format = Printf.bprintf buffer format in
  	begin match error.condition with
  	| `Response response ->
  	let status = Dream.status response in
  	p "%i %s\n" (Dream.status_to_int status) (Dream.status_to_string status)
  	| `String "" ->
  	p "(Library error without description payload)\n"
  	| `String string ->
  	p "%s\n" string
  	| `Exn exn ->
  	let backtrace = Printexc.get_backtrace () in
  	p "%s\n" (Printexc.to_string exn);
  	backtrace |> Dream__middleware.Log.iter_backtrace (p "%s\n")
  	end;
  	p "\n";
  	let layer =
  	match error.layer with
  	| `TLS -> "TLS library"
  	| `HTTP -> "HTTP library"
  	| `HTTP2 -> "HTTP2 library"
  	| `WebSocket -> "WebSocket library"
  	| `App -> "Application"
  	in
  	let blame =
  	match error.caused_by with
  	| `Server -> "Server"
  	| `Client -> "Client"
  	in
  	let severity =
  	match error.severity with
  	| `Error -> "Error"
  	| `Warning -> "Warning"
  	| `Info -> "Info"
  	| `Debug -> "Debug"
  	in
  	p "From: %s\n" layer;
  	p "Blame: %s\n" blame;
  	p "Severity: %s" severity;
  	begin match error.client with
  	| None -> ()
  	| Some client -> p "\n\nClient: %s" client
  	end;
  	begin match error.request with
  	| None -> ()
  	| Some request ->
  	let last = Dream.last request in
  	let major, minor = Dream.version last in
  	p "\n\n%s %s HTTP/%i.%i"
  	(Dream.method_to_string (Dream.method_ last))
  	(Dream.target last)
  	major minor;
  	Dream.all_headers last
  	|> List.iter (fun (name, value) -> p "\n%s: %s" name value);
  	let show_variables kind =
  	kind (fun name value first ->
  	if first then
  	p "\n";
  	p "\n%s: %s" name value;
  	false)
  	true
  	request
  	|> ignore
  	in
  	show_variables Dream.fold_locals;
  	show_variables Dream.fold_globals
  	end;
  	Buffer.contents buffer

  would be useful.

[aantron]

Could this be better addressed by passing the request (if available) to the template? Since Dream is in alpha and will remain so for a while, it's not a problem to add an extra argument if that's the best way to solve an issue.

It seems like all the logging will already have been done, and you can generate the correct response based on the request.

[beajeanm]

If that's on the table, then yes, it would make my error handling implementation trivial 😄

[aantron]

@beajeanm I decided to pass the whole Dream.error object to the template; see here. The request, if available, is in error.request. Using the other Dream.error fields, the template can also access all the information that the surrounding error handler can access.

error.request is a Dream.request option. Even though IIRC all error contexts that end up calling the template do have a request, I didn't want to commit to a request always being available when the template is called. There are other error contexts where there is no request available, and in future development, Dream may end up wanting to call the error template in a situation where there is no way to pass it a request object.

Could you take a look? I assume this is enough :)

[beajeanm]

Yes, that works for me, thanks :)

[aantron]

Ok, it's in master now :)