Fix 401 Unauthorized on log push after token refresh#269
Open
cchristous wants to merge 1 commit intosemaphoreci:masterfrom
Open
Fix 401 Unauthorized on log push after token refresh#269cchristous wants to merge 1 commit intosemaphoreci:masterfrom
cchristous wants to merge 1 commit intosemaphoreci:masterfrom
Conversation
Previously, when a log push request received a 401 Unauthorized response, the agent would refresh the token but then return an error, causing a spurious "Error pushing logs: 401 Unauthorized" message to be logged. The next push would succeed with the new token, but the error log was confusing and happened every hour when tokens expired. This change: - Extracts the HTTP request logic into a separate sendLogRequest function - Stores the buffer contents before sending so they can be reused on retry - After refreshing the token, immediately retries the request instead of returning an error - Limits retry to once to avoid infinite loops The result is cleaner logs: instead of seeing "Successfully refreshed token" followed by "Error pushing logs: 401 Unauthorized", users will see "Retrying log push with refreshed token..." and the request succeeds.
Author
|
I cannot see the test results to debug the failure. I see a 404 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When pushing logs to the Semaphore API, the agent periodically encounters 401 Unauthorized responses due to token expiration. The existing code correctly detects this and calls
RefreshTokenFn()to obtain a new token, but then returns an error instead of retrying the request with the refreshed token.This results in log push failures that look like:
The token was refreshed successfully, but the log batch that triggered the refresh was never retried and is lost until the next push cycle.
Root Cause
In the original
newRequest()function, the HTTP request was created inline. When a 401 was received:l.config.Tokenwas updated with the new tokenThe request body (log buffer) was consumed by the first request attempt and couldn't be reused for a retry.
Solution
buffer.Bytes()before creating the HTTP request so it can be reusedsendLogRequest(): Separate function that can be called recursivelyisRetry=trueisRetryflag ensures we only retry once - a second 401 returns an errorWhy This Is Safe
isRetryboolean prevents infinite retry loops. If the refreshed token also fails, we return an error.RefreshTokenFnpath was already exercised; we're just making use of the new token immediately.Test plan
go test ./...)