fix: distinguish an unreachable backend from expired authentication - #300
Closed
practical-tools-lab wants to merge 2 commits into
Closed
Conversation
Owner
|
Superseded by #301, which implements the same behavior with regression tests and passed all GitHub checks. Thank you @practical-tools-lab for identifying this issue and proposing the original direction. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When the NotebookLM homepage fetch cannot complete, the auth recovery ladder in
base.pyfalls through to the same finalraiseas a genuinely rejected cookie. A transport failure is therefore surfaced as:That message is wrong in this case and actively harmful:
The cause is that layer 1 discards why it failed:
The comment assumes expiry, but
_refresh_auth_tokensraisesValueErrorfor any refresh failure, including one where the request never reached a verdict.Observed
Same profile, same cookies, within the same minute:
nlm login --checkx3network_error: ClientAuthenticationErrornlm notebook listx10login --checkeven prints "your saved credentials may still be valid" while still exiting non-zero, so callers that check the exit code conclude expiry.Change
_is_unreachable_failure()to classify transport failures (httpx.TransportError,httpx.TimeoutException,OSError, and message markers) versus genuine rejection.TransientBackendErrorwhen the evidence only shows an unreachable backend.accounts.google.comredirect or an explicit expiry message) still raisesAuthenticationErrorwith the original text, so existing handlers are unaffected.TransientBackendErroris intentionally not a subclass of the auth error, soexcept AuthenticationErrorcallers no longer swallow a network problem as expiry.Verification
Classifier behavior, run under the installed interpreter:
ValueError("Could not reach NotebookLM (network_error)")ValueError("Authentication expired. accounts.google.com")httpx.ConnectErrorOSError("dns fail")None5/5 as expected. Both touched modules compile cleanly.
Scope
Two files, +55/-5. No behavior change for a real expiry path.