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

fix(log): do not parse error body when response is empty in reloading config #4666

Merged
merged 3 commits into from
Sep 14, 2023

Conversation

randmonkey
Copy link
Contributor

What this PR does / why we need it:

When Kong client fails to get response from reloading declarative configuration (POST /config), do not parse the error body and directly return the error for not getting response, to produce less misleading error log (than could not unmarshal config error: unexpected end of JSON input) for the situations.

Which issue this PR fixes:

fixes #4665

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@randmonkey randmonkey requested a review from a team as a code owner September 14, 2023 07:54
@randmonkey randmonkey self-assigned this Sep 14, 2023
@randmonkey randmonkey added this to the KIC v2.12.0 milestone Sep 14, 2023
programmer04
programmer04 previously approved these changes Sep 14, 2023
@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Patch coverage: 40.0% and no project coverage change.

Comparison is base (2df82cb) 67.8% compared to head (5118c3a) 67.8%.

❗ Current head 5118c3a differs from pull request most recent head 42f8b0b. Consider uploading reports for the commit 42f8b0b to get more accurate results

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #4666   +/-   ##
=====================================
  Coverage   67.8%   67.8%           
=====================================
  Files        164     164           
  Lines      19284   19289    +5     
=====================================
+ Hits       13077   13092   +15     
+ Misses      5435    5426    -9     
+ Partials     772     771    -1     
Files Changed Coverage Δ
...al/dataplane/sendconfig/inmemory_error_handling.go 57.6% <40.0%> (-1.3%) ⬇️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CHANGELOG.md Outdated Show resolved Hide resolved
internal/dataplane/sendconfig/inmemory.go Outdated Show resolved Hide resolved
internal/dataplane/sendconfig/inmemory.go Outdated Show resolved Hide resolved
Copy link
Member

@pmalek pmalek left a comment

Choose a reason for hiding this comment

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

@randmonkey
Copy link
Contributor Author

@pmalek
Copy link
Member

pmalek commented Sep 14, 2023

BTW: when does POST /config return an empty body? When returning a 200? Only? Perhaps we should take the status code into account

@randmonkey
Copy link
Contributor Author

BTW: when does POST /config return an empty body? When returning a 200? Only? Perhaps we should take the status code into account

one situation: When POST /config took too long and client timed out.

@randmonkey randmonkey enabled auto-merge (squash) September 14, 2023 09:49
@randmonkey randmonkey merged commit ac90e1e into main Sep 14, 2023
40 checks passed
@randmonkey randmonkey deleted the fix/error_no_response_from_reload branch September 14, 2023 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants