Improved logging

[KBoehme]

Not sure if I was doing something wrong but it seems like the library could provide more useful logging. For instance I was passing non valid UUID's as the X-Request-ID and in this case all the logging I saw on my server was:

127.0.0.1:51990 - "POST /graphql/ HTTP/1.1" 400

Some descriptive log message would help.

[hhamana]

what would be useful logging?
as we recently saw with the log4j and the jndi debacle, I'm thinking it's best not to log user input, at least not by default, just to be on the safe side. then what would be the added value of a generic "invalid value in header X"?

not quite related, but do note you can now customize the error response sent instead of the default empty 400.

[KBoehme]

Good point about not logging user input. In any case a generic log “X-Request-ID value is bad/invalid uuid” would have saved me a lot of time as it would immediately indicate a place to look and could be an improvement over an empty default 400. I can and probably will look into changing the error message like you said but will also be less likely to get it now that I’m acutely aware of what’s going on.

[hhamana]

right. I guess the lack of any kind of feedback makes it quite hard to debug what's happening for people not familiar with this behavior from a dependency. In terms of discoverability of features, that's pretty poor.
I'm thinking the default response shouldn't be empty, and have some kind of error message already instead. That could at least be some starting point in figuring this out, both for developers and api end users.
Although from a middleware, we cannot raise the Starlette HttpException, we can probably respond something that looks like it, which would be something like that iirc.

{"error": "Invalid header value: X-Request-ID"}

thoughts?

[tomwojcik]

I think we need to distinguish server/internal errors and client errors. The latter ones need to be returned and need to be "loggable".

Let's continue the discussion on the implementation in the PR.

Thanks @KBoehme for raising this.