HTTP headers missing in `NotionHttpClient` debug logging

[scordio]

Currently, NotionHttpClient does not log out HTTP headers in debugLogStart and debugLogSuccess.

Would you be OK with adding them there? I'd be happy to contribute.

As a workaround, I'm currently using a NotionHttpClient decorator where I reimplemented the logging parts.

[seratch]

I intentionally didn’t add headers there because it can be too much noisy plus no benefit by that. Why do you want headers too?

[scordio]

I'm building up integration tests in my project via WireMock and I'm relying on the debug logging of the real communication against a Notion workspace to better analyze what the SDK does and how Notion answers.

Headers are the missing bit, which I'd like to enforce in my test cases.

As an example, here's how a request gets logged with my changes:

POST http://localhost:11501/databases/e5e500c8-da7a-4093-a473-f3c0f9809ceb/query
headers {Authorization=Bearer token, Notion-Version=2022-06-28, User-Agent=notion-sdk-jvm/1.11.1; https://github.com/seratch/notion-sdk-jvm; OpenJDK 64-Bit Server VM/17.0.1; Mac OS X/14.5, Content-Type=application/json; charset=utf-8}
body    {"start_cursor":"aaaaaaaa-bbbb-4b4c-b97a-1ffe9489a10d","page_size":2}

In short, it would be the additional headers line with the map of values.

I can stay with my workaround, in case you prefer not to add them.

[seratch]

Thanks for elaborating. For your use case, I don't think relying on the debug logging outputs is safe enough because it's not structured data, plus this project may change the outputs in future releases. Perhaps, adding a callback mechanism should be more suitable for your use case and others. This means enabling users to pass beforeRequest and afterRequest functions in constructor arguments, which accept as many inputs as possible. This project offers a few HTTP client implementations. Better flexibility while keeping compatibility as much as possible is preferred, but it'd be fine to have a different set of arguments for each implementation (as long as the difference affects only the constructor calls in user code).

[scordio]

For your use case, I don't think relying on the debug logging outputs is safe enough because it's not structured data, plus this project may change the outputs in future releases.

I didn't mean the test would rely on the logging output directly. That's indeed highly fragile. I meant that I'm using the debug logging for manual pre-analysis to set up the test case to be as close to reality as possible.

To be fair, I already see that headers are always the same, so my short-term goal is almost reached 😉

However, I still see an opportunity for extensibility around the NotionHttpClient variants.

Instead of introducing specific beforeRequest and afterRequest functions, what about adding an optional function as a constructor parameter to customize the underlying HTTP client?

For example, OkHttp5Client would execute the given function just before calling client.build(). This would allow attaching custom interceptors. Something similar could be done for the other client variants.