Skip to content

Fixed the "Not Implemented" error code#2828

Merged
tsegismont merged 2 commits intovert-x3:masterfrom
ppatierno:fix-router-builder-not-implemeted-error
Nov 20, 2025
Merged

Fixed the "Not Implemented" error code#2828
tsegismont merged 2 commits intovert-x3:masterfrom
ppatierno:fix-router-builder-not-implemeted-error

Conversation

@ppatierno
Copy link
Copy Markdown
Member

@ppatierno ppatierno commented Nov 18, 2025

This trivial PR fixes the wrong HTTP error code to be returned when an OpenAPI endpoint is "not implemented".
It should be 501 instead of 503 which is "service unavailable" instead.

@ppatierno ppatierno requested a review from tsegismont November 18, 2025 17:29
@ppatierno ppatierno mentioned this pull request Nov 18, 2025
Merged
@vietj vietj added this to the 5.1.0 milestone Nov 18, 2025
tsegismont
tsegismont approved these changes Nov 19, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@tsegismont tsegismont left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you @ppatierno

@tsegismont tsegismont requested a review from pk-work November 19, 2025 09:49
@tsegismont
Copy link
Copy Markdown
Member

tsegismont commented Nov 19, 2025

The compilation failure looks unrelated, I will look into it.

@tsegismont
Copy link
Copy Markdown
Member

tsegismont commented Nov 19, 2025

@pk-work can you please take a look?

I believe the error code is more appropriate.

@tsegismont
Copy link
Copy Markdown
Member

tsegismont commented Nov 19, 2025

When #2829 is merged, this one should be rebased

pk-work
pk-work approved these changes Nov 19, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@pk-work pk-work left a comment

Choose a reason for hiding this comment

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

Good catch!

@pk-work
Copy link
Copy Markdown
Contributor

pk-work commented Nov 19, 2025

I'm wondering if we don't have a test for that. Maybe we should add one

@ppatierno
Copy link
Copy Markdown
Member Author

ppatierno commented Nov 20, 2025

I'm wondering if we don't have a test for that. Maybe we should add one

@pk-work I added a few tests, let me know if they are ok.

@tsegismont
Copy link
Copy Markdown
Member

tsegismont commented Nov 20, 2025

@ppatierno can you please rebase the PR? #2829 has been merged

@ppatierno
Fixed the "Not Implemented" error code
f065903 
Signed-off-by: Paolo Patierno <ppatierno@live.com>
@ppatierno
Added tests for the missing handlers with returned 501 (Not Implemented)
2b59c6b 
Signed-off-by: Paolo Patierno <ppatierno@live.com>
@ppatierno ppatierno force-pushed the fix-router-builder-not-implemeted-error branch from 08bb13e to 2b59c6b Compare November 20, 2025 17:07
@ppatierno
Copy link
Copy Markdown
Member Author

ppatierno commented Nov 20, 2025

@ppatierno can you please rebase the PR? #2829 has been merged

@tsegismont just done! sorry you already said that to me in a previous comment but I forgot :-(

tsegismont
tsegismont approved these changes Nov 20, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@tsegismont tsegismont left a comment

Choose a reason for hiding this comment

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

Thank you @ppatierno !

@tsegismont tsegismont merged commit 1536980 into vert-x3:master Nov 20, 2025
10 of 13 checks passed
@vietj
Copy link
Copy Markdown
Contributor

vietj commented Nov 20, 2025

@tsegismont this is a breaking change and should be listed as is in 5.1 breaking change page

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tsegismont tsegismont tsegismont approved these changes

@pk-work pk-work pk-work approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

5.1.0

Development

Successfully merging this pull request may close these issues.

4 participants

@ppatierno @tsegismont @pk-work @vietj