Skip to content

Conversation

@ds-filipknefel
Copy link

Some HTTP exceptions are assigned custom classes but for an uncovered HTTP code we should propagate the exception.


def wrap_error(e: Exception) -> HTTPException:
if isinstance(e, ingest_errors.UserAuthError):
if isinstance(e, HTTPException):
Copy link
Contributor

Choose a reason for hiding this comment

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

btw this is where its called for HTTPException
https://github.com/Unstructured-IO/unstructured-platform-plugins/blob/main/unstructured_platform_plugins/etl_uvicorn/api_generator.py#L219
do you think we can also move status_code=wrap_error(exc).status_code, -> status_code=exc.status_code?

Copy link
Contributor

Choose a reason for hiding this comment

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

tbh we don't need a case for HTTPException if we don't call this func for it...

Copy link
Contributor

Choose a reason for hiding this comment

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

i created a dup pr #55
for my ticket 😬

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I don't think we need to call the wrap in that case.
I think I'd leave the case for HTTPException though, it makes sense that HTTPException maps onto itself and there's no good way to indicate you shouldn't feed HTTPExceptions into it.

Copy link
Contributor

@yuming-long yuming-long Jun 30, 2025

Choose a reason for hiding this comment

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

hmm no i don't get it tho
if you call wrap_error(e = HTTPException) you would get a CatchAllError(e) which will convert you status code to 512
which is the case for my ticket we see a lot of 512 with emppty detail

@ds-filipknefel
Copy link
Author

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