Conversation
When client.discovery() fails during startup (transient network error, IdP unavailable, temporary HTTP 5xx), the openid passport strategy is never registered and all subsequent OIDC login attempts permanently fail with 'Unknown authentication strategy "openid"'. The server stays up and passes basic HTTP healthchecks, so orchestration (Docker, Kubernetes) cannot detect the broken state. Changes: - Wrap client.discovery() in a retry loop with exponential backoff (2s, 4s, 8s by default) - Validate OPENID_ISSUER URL before retrying — malformed URLs are configuration errors, not transient failures - Configurable via OPENID_DISCOVERY_RETRIES env var (default 3, set to 0 to disable retries) - Separate error messages for discovery failure vs strategy registration failure - Add 4 test cases: transient failure recovery, exhausted retries, zero-retry mode, invalid URL fail-fast Fixs: LibreChat-AI#671, LibreChat-AI#1263, LibreChat-AI#5895, LibreChat-AI#7519, LibreChat-AI#10235, LibreChat-AI#11534 Related: PR LibreChat-AI#9064
| * @param {number} ms | ||
| * @returns {Promise<void>} | ||
| */ | ||
| const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); |
There was a problem hiding this comment.
we have a sleep function already, see other parts of the codebase
There was a problem hiding this comment.
Done — replaced with const { sleep } = require('@librechat/agents'). Matches the pattern used in 8 other files across the codebase. Same signature: (ms) => new Promise((resolve) => setTimeout(resolve, ms)).
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Please address ESLint issues and comment I left. |
|
Addressed both review items in
The pre-existing test suite failure ( |
|
Superseded by #15899, which covers startup retry plus background recovery. Closing. Thanks for the review. |
2 participants
Summary
When
client.discovery()fails during startup (transient network error, IdP unavailable, temporary HTTP 5xx), theopenidpassport strategy is never registered. The server continues running — port listening, healthchecks passing — but all OIDC login attempts permanently fail with:Users see "An unknown error occurred." with no actionable information. A container restart is the only fix.
Root Cause
setupOpenId()inapi/strategies/openidStrategy.jscallsclient.discovery()exactly once. If it throws, the catch logs the error and returnsnull. The passport strategy is never registered. There is no retry and no health signal.Changes
client.discovery(). Configurable viaOPENID_DISCOVERY_RETRIESenv var (default 3, set to 0 to disable).OPENID_ISSUER— invalid URLs are configuration errors, not transient failures. Validated before the retry loop.Backward Compatibility
setupOpenId()still returnsConfiguration | null. The caller insocialLogins.jsalready handles thenullcase. No contract change.Related: #671, #1263, #5895, PR #9064