Skip to content

Conversation

@devin-ai-integration
Copy link
Contributor

📥 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:

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

…#1269

The issue was that upload_logfile() was being called when a trace ended,
but the async authentication task may not have completed yet, causing the
V4 client to not have the auth token set. This resulted in a 401 error.

Changes:
- Check if auth token is available before attempting to upload logfile
- Skip upload gracefully with debug message if auth hasn't completed yet
- Clear buffer even when skipping upload to prevent memory leaks
- Add tests to verify the fix handles both scenarios correctly

This fix ensures that short-running traces that complete before
authentication finishes no longer throw 401 errors.

Co-Authored-By: Alex <[email protected]>
@devin-ai-integration
Copy link
Contributor Author

🤖 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

The tests were calling setup_print_logger() which monkey-patches builtins.print,
but weren't restoring it afterward. This leaked state was causing widespread
CI failures across unit tests and example tests.

Added reset_print fixture to properly restore builtins.print after each test,
mirroring the pattern used in test_instrument_logging.py.

Co-Authored-By: Alex <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: 🖇 AgentOps: [agentops.InternalSpanProcessor] Error uploading logfile: Upload failed: 401

1 participant