Repository navigation
fix(cliproxyapi): sync recovered auth files to R2 on startup #472
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -70,6 +70,29 @@ if [ -n "${OBJECTSTORE_ENDPOINT:-}" ] && [ -n "${OBJECTSTORE_ACCESS_KEY:-}" ]; t | |||||
| fi | ||||||
| fi | ||||||
| fi | ||||||
|
|
||||||
| # CRITICAL: Always sync local auth files to R2 after merge | ||||||
| # This ensures any files that exist locally but not in R2 get uploaded, | ||||||
| # preventing "key does not exist" errors when cliproxyapi reads from object storage | ||||||
| if [ -d "$AUTH_DIR" ] && [ -n "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then | ||||||
| echo "Syncing local auth files to R2..." >&2 | ||||||
| AWS_ACCESS_KEY_ID="${OBJECTSTORE_ACCESS_KEY}" \ | ||||||
| AWS_SECRET_ACCESS_KEY="${OBJECTSTORE_SECRET_KEY}" \ | ||||||
| @aws@ s3 sync \ | ||||||
| --endpoint-url="${OBJECTSTORE_ENDPOINT}" \ | ||||||
| --no-progress \ | ||||||
| "$AUTH_DIR/" \ | ||||||
| "s3://cliproxyapi/auths/" 2>&1 && echo "✅ Auth files synced to R2 auths/" >&2 || echo "⚠️ Failed to sync auth files to R2" >&2 | ||||||
|
|
||||||
| # Also sync to backup location for redundancy | ||||||
| AWS_ACCESS_KEY_ID="${OBJECTSTORE_ACCESS_KEY}" \ | ||||||
| AWS_SECRET_ACCESS_KEY="${OBJECTSTORE_SECRET_KEY}" \ | ||||||
| @aws@ s3 sync \ | ||||||
| --endpoint-url="${OBJECTSTORE_ENDPOINT}" \ | ||||||
| --no-progress \ | ||||||
| "$AUTH_DIR/" \ | ||||||
| "s3://cliproxyapi/backup/auths/" 2>&1 && echo "✅ Auth files synced to R2 backup/" >&2 || true | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the primary sync command, this line redirects stderr to stdout ( To improve logging consistency, I recommend removing the
Suggested change
|
||||||
| fi | ||||||
| else | ||||||
| echo "⚠️ Skipping auth sync: OBJECTSTORE credentials not set" >&2 | ||||||
| fi | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handling for this critical sync operation has a couple of issues:
Incorrect Error Redirection: The use of
2>&1sends theawscommand's standard error to standard output. Given that your service configuration separates stdout and stderr into different log files, this would cause importantawserror messages to be logged in/tmp/cliproxyapi.loginstead of/tmp/cliproxyapi.error.log, making debugging more difficult.Failure Is Not Fatal: The
|| echo ...construct prevents a non-zero exit code from theawscommand from propagating. Sinceset -eis active, a failure in this critical step should terminate the script. However, the current implementation will only print a warning and continue, which could lead to the very "key does not exist" errors this change is intended to fix.My suggestion below corrects both issues by removing the redirection and ensuring the script exits upon failure.