Skip to content

Commit c9fa363

Browse files
tkislanclaude
andcommitted
fix: capture S3 diagnostics and stop logging presigned URLs on SQL cache failures
`upload_sql_cache` reported failures using only the string from `raise_for_status()`. For presigned S3 uploads that discarded the one thing identifying the cause (the response body) while including the one thing that should never reach logs (the presigned URL, which carries X-Amz-Credential and X-Amz-Security-Token). The unique URL in every message also meant no two occurrences shared a message string, so downstream error tracking could not group them. - Extract S3's <Code>/<Message>/<Expires>/<ServerTime> plus x-amz-request-id rather than the exception string. Only those four elements are read; <CanonicalRequest>, <StringToSign> and <AWSAccessKeyId> echo the signed query string and are never logged. - Redact credential-bearing material from any text destined for a log. The primary defence strips the query string off anything URL-shaped, including the scheme-less path-only form urllib3 puts in connection errors - so network failures, not just HTTP ones, are covered. - Carry the URL's issue time in a SqlCacheUpload NamedTuple and log seconds_since_url_issued beside url_expires_in, making an expiry-driven failure self-evident. - Use constant log messages with all variable data in `extra`, at all four sites in the module. - Give serialization failures a distinct `failed_to_serialize_cache` cause, and log only the exception type for them - the message quotes user column names. - Fetch the cached object explicitly instead of handing the URL to pandas, which fetched it with urllib and raised before S3's error body was ever read. The parquet/pickle fallback is preserved and now downloads once instead of twice. - Bound the connect phase of both transfers at 5s; read is left unbounded so slow but healthy transfers of large cache objects are unaffected. Cache failures remain swallowed and can never fail the user's query. tests/unit/test_sql_caching.py goes from 26 tests in 31.1s to 74 in 1.4s - the former suite had a test making a real network call to localhost:19456 whose mocks were provably dead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015xJczfWE4mq3ux7ZU9ZeU9
1 parent 6198101 commit c9fa363

3 files changed

Lines changed: 1183 additions & 78 deletions

File tree

0 commit comments

Comments
 (0)