Minor : Use specific error in compression catch handle #20144
Conversation
github-actions
bot
added
triage
| @@ -717,7 +718,7 @@ private Optional<Builder<?>> createPushRequest(ClientTelemetrySubscription local | |||
| ByteBuffer compressedPayload; | |||
| try { | |||
| compressedPayload = ClientTelemetryUtils.compress(payload, compressionType); | |||
| } catch (Throwable e) { | |||
| } catch (IOException | NoClassDefFoundError e) { | |||
| log.debug("Failed to compress telemetry payload for compression: {}, sending uncompressed data", compressionType); | |||
There was a problem hiding this comment.
Perhaps the log could include the exception?
There was a problem hiding this comment.
What about the compressionType in the next call i.e. once NoClassDefFoundError error is encountered then it will happen for every push hence do we want to try other compression types as supported by broker or move to compression type none for next push?
There was a problem hiding this comment.
@chia7712 Addressed in comment 7f258c8
@apoorvmittal10
That makes sense! Can this functionality be added in seperate PR?
There was a problem hiding this comment.
I don't think there is any value in having a separete PR and we can't achieve the right way in this PR itself, we should create a jira and fix it. Else if there is something else for which you need this PR to go early then it's different, and I guess there is nothing as this new behaviour will anyways go in 4.2.
There was a problem hiding this comment.
@apoorvmittal10
We need to merge this PR first, to fix this workaround for users which are facing this problem in older versions of connect and schema registry jars .
There was a problem hiding this comment.
@k-raina catching the Throwable is not a major issue, so there is no hurry to include it in the 4.1.0 release. Perhaps you could address @apoorvmittal10 comment in this PR, and then we could backport it to 4.1.1
| @@ -717,7 +718,7 @@ private Optional<Builder<?>> createPushRequest(ClientTelemetrySubscription local | |||
| ByteBuffer compressedPayload; | |||
| try { | |||
| compressedPayload = ClientTelemetryUtils.compress(payload, compressionType); | |||
| } catch (Throwable e) { | |||
| } catch (IOException | NoClassDefFoundError e) { | |||
| log.debug("Failed to compress telemetry payload for compression: {}, sending uncompressed data", compressionType); | |||
There was a problem hiding this comment.
What about the compressionType in the next call i.e. once NoClassDefFoundError error is encountered then it will happen for every push hence do we want to try other compression types as supported by broker or move to compression type none for next push?
Instead of throwable, use specific error in compression catch handle