agentops icon indicating copy to clipboard operation
agentops copied to clipboard

Fix: Handle missing auth token gracefully in logfile upload - Resolves #1269

Open devin-ai-integration[bot] opened this issue 2 months ago โ€ข 1 comments

๐Ÿ“ฅ 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:

  • Added auth token check in upload_logfile() before attempting upload
  • Skip upload gracefully with debug message if auth not complete
  • Clear buffer in both success and skip scenarios
  • Added unit tests for both authenticated and unauthenticated scenarios

๐Ÿงช Testing

  1. Unit Tests: Added test_upload_logfile_no_auth.py with two test cases:

    • test_upload_logfile_skips_when_no_auth_token() - Verifies upload is skipped when auth token is None
    • test_upload_logfile_uploads_when_auth_token_is_set() - Verifies upload proceeds normally when auth token is set
    • Both tests pass โœ…
  2. Lint Checks: All ruff checks pass โœ…

๐Ÿ” Review Focus Areas

โš ๏ธ Important: Please review the following:

  1. 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?

  2. Logging Level: Currently using logger.debug(). Should this be logger.warning() or logger.info() instead to make it more visible?

  3. 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.

  4. Thread Safety: The auth token check if not client.api.v4.auth_token assumes this is thread-safe with the async auth task. Review the Client class implementation to confirm.


Session Info:

  • Link to Devin run: https://app.agentops.ai/sessions/85ded112b5314bb49ef3bb8b99d26f12
  • Requested by: Alex ([email protected]) / @areibman

Devin Session: https://app.devin.ai/sessions/85ded112b5314bb49ef3bb8b99d26f12

๐Ÿค– Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

โœ… I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

โš™๏ธ Control Options:

  • [ ] Disable automatic comment and CI monitoring