Fix: Handle missing auth token gracefully in logfile upload - Resolves #1269 #1270
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.
📥 Pull Request
📘 Description
Fixes #1269 - Resolves 401 error when uploading logfiles for short-running traces.
Root Cause: The
upload_logfile()function was being called when traces ended, but the async authentication task running in the background may not have completed yet. This resulted in the V4 client not having an auth token set, causing a 401 Unauthorized error.Solution: Added a check to verify the auth token is available before attempting to upload the logfile. If authentication hasn't completed yet, the upload is skipped gracefully with a debug message instead of throwing a 401 error. The log buffer is still cleared to prevent memory leaks.
This is particularly common for short-running traces that complete before the async authentication finishes (typically takes a few hundred milliseconds).
Changes:
upload_logfile()before attempting upload🧪 Testing
Unit Tests: Added
test_upload_logfile_no_auth.pywith two test cases:test_upload_logfile_skips_when_no_auth_token()- Verifies upload is skipped when auth token is Nonetest_upload_logfile_uploads_when_auth_token_is_set()- Verifies upload proceeds normally when auth token is setLint Checks: All ruff checks pass ✅
🔍 Review Focus Areas
Behavior Change: The fix changes behavior from throwing a 401 error to silently skipping with a debug log. Is this the desired behavior, or should we retry/queue the upload instead?
Logging Level: Currently using
logger.debug(). Should this belogger.warning()orlogger.info()instead to make it more visible?Buffer Clearing: The fix clears the buffer even when skipping upload (to prevent memory leaks). This means logs from unauthenticated sessions are lost. Confirm this is acceptable.
Thread Safety: The auth token check
if not client.api.v4.auth_tokenassumes this is thread-safe with the async auth task. Review the Client class implementation to confirm.Session Info:
Devin Session: https://app.devin.ai/sessions/85ded112b5314bb49ef3bb8b99d26f12