Skip to content

Report non-JSON token-endpoint responses as BfabricOAuthError - #601

Open
Caushi wants to merge 1 commit into
mainfrom
fix/device-code-non-json-error
Open

Report non-JSON token-endpoint responses as BfabricOAuthError#601
Caushi wants to merge 1 commit into
mainfrom
fix/device-code-non-json-error

Conversation

@Caushi

@Caushi Caushi commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Fix device-code login to report a non-JSON token-endpoint response (e.g. an app-server 404 page while the REST module redeploys) as BfabricOAuthError, so the CLI prints an error message instead of an httpx.HTTPStatusError traceback.
Correct two stale entries in the OAuth design doc: from_url_token() and parse_url_token() do not exist — the real API is connect_token() / from_token_data() and verify_jwt().
Add the missing connect_pat() row to the design doc's factory-method table.

…rror

Device-code polling parsed the token endpoint's error body as JSON to tell
authorization_pending from a real failure. When the body was not JSON at all
-- an app-server 404 page while the REST module was redeploying -- the handler
called raise_for_status(), throwing httpx.HTTPStatusError. That is neither a
ValueError nor a RuntimeError, so it escaped @use_client's handler and the
user got a traceback instead of an error message.

Raise the domain error directly, so the CLI reports it like any other OAuth
failure. Found while validating the OAuth flows against bfabric-test: the
device code was authorized just as the instance redeployed.

Also correct two stale entries in the OAuth design doc: from_url_token() and
parse_url_token() do not exist -- the real API is connect_token() /
from_token_data() and verify_jwt() -- and add the missing connect_pat() row.
@Caushi
Caushi force-pushed the fix/device-code-non-json-error branch from efce4ef to f7c1a5c Compare August 14, 2026 08:54
@leoschwarz

Copy link
Copy Markdown
Member

Thank you. I see no problem merging this, but I have two more general questions:

  1. Would it make sense to handle the errors when bfabric is unavailable generically? Because I don't think this is properly handled in the rest of bfabricPy either, actually I know of at least one service which creates problems when that happens to prod.
  2. Would it make sense that we check in general, that OAuth is available, e.g. by checking .well-known/openid-configuration? Since this is not really adding any security something like a /health endpoint would also do it.

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.

2 participants