Skip to content

fix: return response body from revoke! for logging pipeline#562

Merged
viacheslav-rostovtsev merged 3 commits intogoogleapis:mainfrom
katsu223:main
Feb 26, 2026
Merged

fix: return response body from revoke! for logging pipeline#562
viacheslav-rostovtsev merged 3 commits intogoogleapis:mainfrom
katsu223:main

Conversation

@katsu223
Copy link
Contributor

Fixes a bug in UserRefreshCredentials.revoke! where the method was returning an integer (0) instead of the response body, causing JSON parsing errors in log_response.

Fixes: #540

retry_with_error passes the block's return value to log_response,
which expects a JSON-parseable string.
@katsu223 katsu223 requested a review from a team February 12, 2026 19:38
@viacheslav-rostovtsev viacheslav-rostovtsev requested a review from a team as a code owner February 24, 2026 02:13
@viacheslav-rostovtsev
Copy link
Member

Hello! Thank you for raising the issue and implementing a fix. While looking into it I've also noticed that the log_response method itself never invokes the logger. I've changed that (will now log at debug) and pushed a commit to your branch (there are also tests for logging and execution speedups).

LMK if you would prefer me to do this in a separate PR rather than yours

@katsu223
Copy link
Contributor Author

Hello! Thank you for raising the issue and implementing a fix. While looking into it I've also noticed that the log_response method itself never invokes the logger. I've changed that (will now log at debug) and pushed a commit to your branch (there are also tests for logging and execution speedups).

LMK if you would prefer me to do this in a separate PR rather than yours

Hello. Thank you for taking an eye on that. I personally don't mind, you may leave changes here

@viacheslav-rostovtsev viacheslav-rostovtsev merged commit fb5b797 into googleapis:main Feb 26, 2026
17 of 19 checks passed
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.

UserRefreshCredentials.revoke! block returns 0 which causes revoke to fail

3 participants