armeria icon indicating copy to clipboard operation
armeria copied to clipboard

Expose unexpected decoding exception reason

Open Be-poz opened this issue 10 months ago • 5 comments

Motivation:

Issue: https://github.com/line/armeria/issues/5177

Modifications:

  • I added UnexpectedDecodeException to throw an exception if a DecompressionException is thrown and the if condition is not met.
  • Added tests ( I decided that it is important that the decoder throws a DecompressionException and that understanding the internal implementation of the decoder is not what i am testing, so i decided to use a mock at the first time. However, it was difficult to use 'mock' because the 'decoder' variable is created and initialized in the constructor instead of being injected, and it was difficult to manipulate it using inheritance because the access modifier level of the constructor of AbstractStreamDecoder is 'protected'. Therefore, I added two separate classes for testing. I don't think this structure is efficient, and I think it's hard to manage when AbstractStreamDecoder class changes are made. I couldn't think of any other way to do it, so I went with this, but if anyone has a better idea, I'd love to hear it.

Result:

  • Closes https://github.com/line/armeria/issues/5177

Be-poz avatar Apr 18 '24 13:04 Be-poz

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Apr 18 '24 13:04 CLAassistant

🔍 Build Scan® (commit: e3da519403d0619b237906658b49a08e90d46700)

Job name Status Build Scan®

github-actions[bot] avatar Apr 18 '24 14:04 github-actions[bot]

And please sign the ICLA :bow:

trustin avatar Apr 23 '24 13:04 trustin

I've signed ICLA. Sorry for missed this.

Be-poz avatar Apr 25 '24 13:04 Be-poz

I've applied trustin's review. left one jrhee's comment.

Be-poz avatar May 07 '24 13:05 Be-poz

Revised and cleaned up the PR description and commit message, which was previously out of sync.

trustin avatar Jun 20 '24 14:06 trustin