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

Add support in Pekko interpreter for status codes >=600 #4322

Conversation

gkepka
Copy link
Contributor

@gkepka gkepka commented Feb 6, 2025

Pekko supports registering custom status codes with values >=600. Currently, if an endpoint returns such a status code, the Pekko interpreter will crash due to the call to the two argument version of StatusCodes.custom. This PR adds possibility of registering Pekko custom status codes using PekkoHttpServerOptions fixes that behavior by using the five parameter version of the method.

Comment on lines 3 to 4
//> using dep com.softwaremill.sttp.tapir::tapir-core:1.11.13
//> using dep com.softwaremill.sttp.tapir::tapir-pekko-http-server:1.11.13
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is obviously not correct, but I don't know which version to use in an example for a feature that has not been released yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately examples of new features have to be commented out until there's a release

@adamw
Copy link
Member

adamw commented Feb 6, 2025

As I understand you need to call the StatusCodes.custom(800, "Custom reason", "Custom message", isSuccess = false, allowsEntity = true) constructor - maybe we can simply always do that, if the status code is not found? Always allowing an entity etc. - I don't think that information is used anywhere anyhow, as it's simply used to send the response? and these flags don't seem to influence that process

@gkepka gkepka force-pushed the add-custom-status-codes-support-for-pekko-interpreter branch from c70ed7f to 6735172 Compare February 6, 2025 12:28
@gkepka
Copy link
Contributor Author

gkepka commented Feb 6, 2025

For my use case it will be enough. I haven't checked the Pekko HTTP internals very carefully, but judging from a glance parameters like reason or defaultMessage seem to be used for handling responses only. It should be safe to leave them empty.

@gkepka gkepka changed the title Add support for registering Pekko custom status codes Add support in Pekko interpreter for status codes >=600 Feb 6, 2025
@adamw
Copy link
Member

adamw commented Feb 6, 2025

Maybe add a test? :)

@adamw
Copy link
Member

adamw commented Feb 6, 2025

PekkoHttpServerTest might be a good place

@adamw adamw merged commit e286708 into softwaremill:master Feb 6, 2025
27 checks passed
@adamw
Copy link
Member

adamw commented Feb 6, 2025

Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants