Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider making ContentTooLargeException extend HttpStatusException #5578

Closed
ikhoon opened this issue Apr 8, 2024 · 2 comments
Closed

Consider making ContentTooLargeException extend HttpStatusException #5578

ikhoon opened this issue Apr 8, 2024 · 2 comments
Labels

Comments

@ikhoon
Copy link
Contributor

ikhoon commented Apr 8, 2024

Suggested by @dlvenable (#5565)

The ContentTooLargeException does not inherit from HttpStatusException. We currently have a block of code that handles HttpStatusException exceptions by looking to see if it is a client or server error. Can we make ContentTooLargeException inherit from HttpStatusException?

ContentTooLargeException represents 413 status so, I think, we can make ContentTooLargeException extend HttpStatusException to streamline exception handling logic.

@ikhoon
Copy link
Contributor Author

ikhoon commented Apr 22, 2024

I realized that ContentTooLargeException can't simply extend HttpStatusException because HttpStatusException is in server package but ContentTooLargeException is in common.

It seems like this issue should be considered in Armeria 2.0.

@minwoox minwoox removed this from the 2.0.0 milestone May 22, 2024
@minwoox
Copy link
Member

minwoox commented May 22, 2024

I believe that we don't want to make ContentTooLargeException extend HttpStatusException.
See this comment: #5565 (comment)

@minwoox minwoox closed this as completed May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants