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

[LLT-4202] Make lookup return error in case of failure on forward #2

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

dfetti
Copy link

@dfetti dfetti commented Oct 30, 2023

Make the forward error visible instead of just filling the response buffer with an empty response.

@dfetti dfetti force-pushed the LLT-4202-return-error-if-forward-fails branch from 77d0fae to f9c2ba7 Compare October 30, 2023 07:51
@dfetti dfetti force-pushed the LLT-4202-return-error-if-forward-fails branch 3 times, most recently from ffcddd5 to 03e6c3c Compare November 7, 2023 11:43
@matszczygiel
Copy link

+1

@mathiaspeters
Copy link

The commented out workflows should be commented why they're commented out and if there's isn't already there should be a ticket to fix it, or an issue opened upstream that can be linked in the aforementioned comment. It might make sense to add the comment both in the code and in the commit message

@tomaszklak
Copy link

The commit message should also explain why we want to make lookup return error in case of failure on forward.

Make the forward error visible instead of just filling the response
buffer with an empty response.

The inieal problem was that if the forward lookup from trust-dns fails,
the failure is not indicated to libtelio. Instead the response buffer
was filled with an empty lookup response. This, combined with the
doubled retry mechanis (OS and trust-dns) produced some issues on IOS.

This commit also removes the compatibility checks from the workflows. It
started to fail after the version update. I created LLT-4544 to fix it.
@dfetti dfetti force-pushed the LLT-4202-return-error-if-forward-fails branch from 03e6c3c to c87d453 Compare November 15, 2023 01:45
Copy link

@tomaszklak tomaszklak left a comment

Choose a reason for hiding this comment

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

+1.0

@dfetti dfetti merged commit f3583ef into release/0.24.0 Nov 15, 2023
16 checks passed
@dfetti dfetti deleted the LLT-4202-return-error-if-forward-fails branch November 15, 2023 12:31
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.

4 participants