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

Create IsDownstreamHTTPError method and add error source middleware #1102

Closed
wants to merge 7 commits into from

Conversation

ivanahuckova
Copy link
Member

@ivanahuckova ivanahuckova commented Oct 3, 2024

This PR:

  • Creates backend.IsDownstreamHTTPError method that can be used by plugins to check if returned HTTP error is downstream
  • Adds error source middleware to default middlewares that checks if error is downstream and if so, adds source for it

There was an issue with circular dependencies. I initially tried to move some stuff to backend/error_source.go or backend/experimental/errorsource, but none of these can be imported in http client, because:

  • backend imports httpclient and therefore we can't import backend in httpclient
  • experimental/errorsource imports both backend and httpclient

But then landed with the current solution. Let me know what do you think.

@ivanahuckova ivanahuckova requested a review from a team as a code owner October 3, 2024 16:04
@ivanahuckova ivanahuckova requested review from wbrowne, marefr and andresmgot and removed request for a team October 3, 2024 16:04
@ivanahuckova ivanahuckova marked this pull request as draft October 3, 2024 16:05
@ivanahuckova ivanahuckova changed the title IsDownstreamHttpError IsDownstreamHttpError + error source middleware Oct 3, 2024
@ivanahuckova ivanahuckova changed the title IsDownstreamHttpError + error source middleware POC: IsDownstreamHttpError + error source middleware Oct 3, 2024
@marefr
Copy link
Member

marefr commented Oct 7, 2024

@ivanahuckova ivanahuckova changed the title POC: IsDownstreamHttpError + error source middleware Create IsDownstreamHTTPError method and add error source middleware Oct 8, 2024
@ivanahuckova ivanahuckova marked this pull request as ready for review October 8, 2024 13:37
@marefr
Copy link
Member

marefr commented Oct 10, 2024

@ivanahuckova Opened an alternative I would like to explore #1106. Found it too hard to suggest those changes within this PR, sorry. We'll see if that allows to keep backward compatibility and in that case I think it's an interesting idea 🙏

@ivanahuckova
Copy link
Member Author

Closed in favour of #1102

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