feat: sentry add gzipped with compression

[Strive-Sun]

Hi, I noticed that the http request from sentry is missing content-encoding, so I added this parameter. DeflateInit2 references header file in crashpad, and I am not sure if this reference is appropriate. if further modifications are needed, please let me know at any time. Thx in advance.

[Swatinem]

What is missing here is building / linking the zlib code

[Strive-Sun]

There is a new question, if diable SENTRY_TRANSPORT_COMPRESSION, sentry_transport.c will complie failed on due to missing zlib.lib. Is there a macro that does not compile sentry_gzipped_with_compression function and zlib.h.

[supervacuus]

There is a new question, if diable SENTRY_TRANSPORT_COMPRESSION, sentry_transport. c will complie failed on due to missing zlib.lib. Is there a macro that does not compile sentry_gzipped_with_compression function and zlib.h.

It would be best if you created a compile definition in the CMakeLists.txt and #ifdef that definition in the transport module. Specifically, you must include the zlib header and your entire compression code only when that definition exists. Otherwise, users without zlib developer packages will have failed builds.

Given that sentry__gzipped_with_compression is only called from within sentry_transport.c, exposing it in a header is unnecessary. Please,

• remove the header declaration,
• remove the sentry__ prefix from the function name,
• move the function before its first use and
• make the function static

Thx!

[Strive-Sun]

There is a new question, if diable SENTRY_TRANSPORT_COMPRESSION, sentry_transport. c will complie failed on due to missing zlib.lib. Is there a macro that does not compile sentry_gzipped_with_compression function and zlib.h.

It would be best if you created a compile definition in the CMakeLists.txt and #ifdef that definition in the transport module. Specifically, you must include the zlib header and your entire compression code only when that definition exists. Otherwise, users without zlib developer packages will have failed builds.

Given that sentry__gzipped_with_compression is only called from within sentry_transport.c, exposing it in a header is unnecessary. Please,

• remove the header declaration,
• remove the sentry__ prefix from the function name,
• move the function before its first use and
• make the function static

Thx!

Great idea, please help me review again. Thx a lot.

[codecov]

Codecov Report

Attention: Patch coverage is 28.57143% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 82.68%. Comparing base (fda2a37) to head (42ebfb5).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #954      +/-   ##
==========================================
+ Coverage   82.64%   82.68%   +0.04%     
==========================================
  Files          53       53              
  Lines        7444     7450       +6     
  Branches     1198     1199       +1     
==========================================
+ Hits         6152     6160       +8     
+ Misses       1182     1179       -3     
- Partials      110      111       +1     

[supervacuus]

This is a quick heads-up, @Strive-Sun: the error in the MinGW build is unrelated to this PR (the fact that it is zlib is a coincidence). I'll find out what is going on and continue my review tomorrow. Thx.

[supervacuus]

The CMake failure to find zlib in the MinGW build is due to a change in the GHA runner image. In previous builds, zlib was found in the StrawberryPerl installation 😅 .

I will provide a fix for this, which the open PRs can rebase on, next week.

[supervacuus]

A few minor changes are still necessary. Thanks for your work so far.

Integration tests that validate the setting on supported platforms would be great, but I can add those after merging this PR.

[Strive-Sun]

Hi, I have rebased #964. Please help review again, thx a lot

[supervacuus]

We can soon merge this. Only minor additions, please

• add a change-log
• wrap find_package() in the crashpad build
• apply the suggestions with MAX_HTTP_HEADERS (or tell me why that is a bad idea 😸)
• please rebase once more because yet another GHA runner image update introduced an ASAN issue (fix: failing clang-asan/llvm-cov tests #965).

Thx!

I will take care of an integration test in a follow-up PR, because it might also require a few changes to the CI config.

[supervacuus]

Thanks a lot for your contribution @Strive-Sun!

[Strive-Sun]

Thanks a lot for your contribution @Strive-Sun!

My pleasure. Without your suggestions and help, I wouldn't have been able to complete it smoothly.😄