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

Allow specifying/retaining headers when injecting HTTP faults. #185

Open
Tracked by #251
pablochacin opened this issue Jun 6, 2023 · 4 comments
Open
Tracked by #251

Allow specifying/retaining headers when injecting HTTP faults. #185

pablochacin opened this issue Jun 6, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@pablochacin
Copy link
Collaborator

When injecting HTTP faults, the response from the proxy does not carry any header (besides the mandatory Content-lenght).
In some situations, it is convenient to add headers such as the Content-Type (for example, when returning an error body).

Additionally, it worth investigating if in some cases the client expects some headers sent to the server to be also included in the response (for example, transactions ids) and if so, consider a mechanism for supporting this (for example, using "header-name: *" to indicate the header-name header must be returned in the response if present in the request.

@pablochacin pablochacin added the enhancement New feature or request label Jun 6, 2023
@nadiamoe
Copy link
Member

nadiamoe commented Jun 6, 2023

A possible API coming to my mind could be:

const fault = {
  errorCode: 500,
  errorRate: 0.1,
  errorBody: '{"this": "that"}',
  errorHeaders: {"This-Is": "A hardcoded header"},
  passthroughHeaders: ["Trace-Id"]
};

Perhaps we could allow wildcards in passthroughHeaders: passthroughHeaders: ["*"]. The logic should take into account that if len(passthroughHeaders) != 0 the request should be made anyway, and the body discarded.

@nadiamoe
Copy link
Member

nadiamoe commented Jun 6, 2023

A big rock for retaining/passing through headers are non-GET requests. GET requests are strongly encouraged to be free of side-effects, but any other type is almost certainly not.

To be able to pass headers through, the proxy would need to perform the real request to upstream, but it will then tell the client that the request did not succeed. For POST, PUT and PATCH requests this may trigger retries that may later on fail for different reasons (e.g. attempting to create an object that already exists, or even worse, creating duplicates).

I see two ways out of this:

  1. Implement only static errorHeaders, preventing these cases entirely with clean UX but reduced functionality.
  2. Implement passthroughHeaders only for GET requests, ignoring them for others. This would have a worse UX, as it is not immediately obvious why.

@pablochacin
Copy link
Collaborator Author

@roobre all those are very valid points.

I think we should probably avoid over-engineering or prematurely optimizing the first iteration of the solution and only implement the explicit error headers and take time to better understand the requirements for passthrough headers based on experiences with actual systems using the distuptor.

@nadiamoe
Copy link
Member

nadiamoe commented Jun 7, 2023

Agreed, I think the static headers should cover potential use cases for Content-Type and so on. If a strong use case requiring dynamic headers rises, we can think what to do with it then.

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

No branches or pull requests

2 participants