-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Elastic Agent] Fixed status reporting #25584
Conversation
Pinging @elastic/agent (Team:Agent) |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪💚 Flaky test reportTests succeeded. Expand to view the summary
Test stats 🧪
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I haven't tested it yet.
Also, test failure is going to be fixed by: #25587 |
Tested, works well. |
switch e := err.(type) { | ||
case apiError: | ||
w.WriteHeader(e.Status()) | ||
default: | ||
w.WriteHeader(http.StatusInternalServerError) | ||
|
||
} | ||
|
||
writeResponse(w, unexpectedErrorWithReason(err.Error())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible to add a unit test for this code path? Having a unit test would have been an easy way to catch this.
[Elastic Agent] Fixed status reporting (elastic#25584)
[Elastic Agent] Fixed status reporting (elastic#25584)
What does this PR do?
Fixed some issues discovered on testing.
First one is not to use overwrites when in standalone mode second one is that status code was always 200. (written after body)
Why is it important?
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.