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

Updating exceptions for 404 to include authority #5095

Merged
merged 8 commits into from
Jan 31, 2025

Conversation

trwalke
Copy link
Member

@trwalke trwalke commented Jan 22, 2025

Fixes #
Fix for #4769

Changes proposed in this request

Authority will be appended to exception messages when 404 is encountered.

Testing

Unit.

Performance impact

Documentation

  • All relevant documentation is updated.

@trwalke trwalke requested a review from a team as a code owner January 22, 2025 07:59
Copy link
Member

@bgavrilMS bgavrilMS left a comment

Choose a reason for hiding this comment

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

Possible NPEs

@trwalke
Copy link
Member Author

trwalke commented Jan 22, 2025

Using the endpoint was a mistake, however, do we want to add it anyway?, alongside the authority in the exception for 404?

Copy link
Contributor

@gladjohn gladjohn 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 @trwalke

@bgavrilMS
Copy link
Member

This has been signed off @trwalke - please merge it

@trwalke
Copy link
Member Author

trwalke commented Jan 31, 2025

This has been signed off @trwalke - please merge it

@bgavrilMS the builds were all failing since Monday, was waiting for them to be fixed. Seems like they are working now. Merging when pr build is green

@trwalke trwalke merged commit 79de40f into main Jan 31, 2025
6 checks passed
@trwalke trwalke deleted the trwalke/404AuthorityException branch January 31, 2025 08:01
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.

3 participants