No longer close OutputStream that is passed into JsonSerializer

[adinauer]

📜 Description

No longer (auto) close the OutputStream that is passed into JsonSerializer. This caused problems when passing in System.out from StdoutTransport as it closed the stream.

💡 Motivation and Context

Fixes #2027 for 6.x

💚 How did you test it?

• Unit Test
• code sample

📝 Checklist

• I reviewed the submitted code
• I added tests to verify the changes
• I updated the docs if needed
• No breaking changes

🔮 Next steps

[codecov-commenter]

Codecov Report

Merging #2029 (80a7b18) into 6.x.x (f60ace2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              6.x.x    #2029   +/-   ##
=========================================
  Coverage     80.79%   80.80%           
- Complexity     3146     3147    +1     
=========================================
  Files           228      228           
  Lines         11645    11645           
  Branches       1565     1565           
=========================================
+ Hits           9409     9410    +1     
+ Misses         1649     1648    -1     
  Partials        587      587           

Impacted Files	Coverage Δ
sentry/src/main/java/io/sentry/JsonSerializer.java	97.91% <100.00%> (ø)
...ry/src/main/java/io/sentry/SentryEnvelopeItem.java	87.41% <0.00%> (+0.69%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f60ace2...80a7b18. Read the comment docs.

[romtsn]

LGTM 👍 Just one idea maybe - not sure if it'd make sense to introduce another method overload, which accepts a boolean like autoClose? Probably not needed though

[marandaneto]

should be likely in the finally block?

[adinauer]

Only moved the code around. There's a catch block that only logs but doesn't throw out right before though.

[marandaneto]

LGTM 👍 Just one idea maybe - not sure if it'd make sense to introduce another method overload, which accepts a boolean like autoClose? Probably not needed though

I think that this is actually needed, since the streams will be open or will wait to be GCed, we could rather make it closable by default, and the StdoutTransport transport pass true to this param when calling the serialize method.

[romtsn]

I've checked the usages of this method - basically everywhere we use try-with-resources around it, so we are safe internally. But yeah, if we expose this to the consumers, this may come as a surprise.

[adinauer]

We also don't have an option to close the writer that is passed into public <T> void serialize(@NotNull T entity, @NotNull Writer writer) throws IOException {.

Also the NoOpSerializer would then also have to close the stream to be consistend and not leave peoples streams open despite them telling us to close via the boolean you suggested.

[adinauer]

@marandaneto @romtsn do you still want me to implement a flag for autoClose after my explanation above?

[romtsn]

I'm good without it, since the writer overload does not close by default already anyway, so we're just aligning here kinda. Internally we're closing it everywhere, so we're good. For the consumers - I hope the javadoc is enough