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

[Http Retry] Response headers interference #269

Open
joepkalthoff opened this issue Sep 4, 2024 · 2 comments
Open

[Http Retry] Response headers interference #269

joepkalthoff opened this issue Sep 4, 2024 · 2 comments
Milestone

Comments

@joepkalthoff
Copy link

joepkalthoff commented Sep 4, 2024

Summary

Retries in the Http component include headers from the previous (re)try.

Testcase

Dovetail version: 4.17.0-SNAPSHOT-dcb7cc8
Instance: next
Tenant: Regression Tests

Sending Flow: HttpRetrySocketAsynchronous
Flow version: 7
url: https://next.dovetail.world/flowdesigner/66b1efbde56ba4001000012e/7/route

Receiving Flow: HttpRetry500
Flow version: 1
url: https://next.dovetail.world/flowdesigner/66b31c0ee56ba4000e00002f/1/route

Input:

Output:
Notice the transactions of the Sending flow and the Receiving flow. Especially the body and headers for the first try and the 2 retries. There are screenshots in the results section below:

Result

Initial Http request

Sending
Image

Receiving
Image

1st retry

Sending
Image

Receiving
Image

2nd retry

Sending
Image

Receiving
Image

After the Http retries

Image

Conclusion

I expect the headers from the 5xx responses to not interfere with the retry messages. Only the breadcrumbId, Content-Type and counter header should be included.

There should be a way to see the response headers somewhere though. They can contain valuable information about why the server responded with a 5xx in the first place. Maybe in the Logs of a flow?

The same is true for the 5xx response body. This is a seperate issue: #268.

Original issue on Dovetail Application board: https://github.com/dovetailworld/front-end/issues/4635.

@skin27
Copy link
Member

skin27 commented Sep 13, 2024

There is already a mechanism that logs all relevant information. I assume this isn't probably referenced to the correct flow, so that it's not in log of the flow (only in the general log on the server). We will investigate this.

@skin27
Copy link
Member

skin27 commented Sep 13, 2024

I investigated this, and largely this is by design:

  1. There is a retry step, but we explicitly filter this out of the transactions as this is not seen on the canvas.
  2. The headers are largely removed, because otherwise this will cause issues when resending
  3. The headers/body etc are not logged, because when a message can be resend 100s of times. When we have big messages or large headers, this can fill the log directory very fast on the server.

As this is resend and not send to the error route, it's logged as a warning:

"HTTP Status: 500 Server Error | Flow: ID_66b1efbde56ba4001000012e | API Endpoint: https://next.dovetail.world/test/inbound_http/regressiontests/HttpRetry500 | Retry in 10000 milliseconds | Attempt 1 "

This gives the following warning:

  1. HTTP Error and status code
  2. Flow ID
  3. The Endpoint is calling
  4. Retry interval
  5. The attempt

This is minimal because when there is a 500, this means there is "an internal server error", which means the error won't give any extra useful information I guess, and then it just retries.

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

No branches or pull requests

3 participants