Skip to content
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

UserResource.update: provide error details #140

Open
lrozenblyum opened this issue Jan 17, 2025 · 2 comments
Open

UserResource.update: provide error details #140

lrozenblyum opened this issue Jan 17, 2025 · 2 comments

Comments

@lrozenblyum
Copy link

lrozenblyum commented Jan 17, 2025

Description

Improve the signature of org.keycloak.admin.client.resource.UserResource.update(UserRepresentation) by returning the Response object or providing details in the exception object.

Discussion

No response

Motivation

In case of an error (e.g. bad request 400) the backend returns a JSON that helps identifying the cause of the error (e.g.

{
    "field": "username",
    "errorMessage": "error-user-attribute-read-only",
    "params": [
        "username"
    ]
}

yet the Admin Client doesn't return it/doesn't provide the details in the exception.

A similar call for user creation (org.keycloak.admin.client.resource.UsersResource.create(UserRepresentation)) does return the Response

Details

No response

@mposolda
Copy link
Contributor

@lrozenblyum I think that this is on purpose. AFAIK we ideally don't want to return Response from as the returned Response hides the fact that request failed.

For this one:

  • If the call to userResource.update successfully passed with HTTP status 200, the method on admin-client will finish successfully (and returning void)
  • If the call to userResource.update failed for some reason, it will throw the exception (for example BadRequest exception in case of 400 or NotFoundException in case of 404 or Forbidden exception in case of 403 etc).

IMO the exception is better than returning Response (the "create" called never throws exception, but caller need to check the status code).

We may revisit this when starting on Admin API v2. But I don't think that we're going to change in current admin API. BTW. I believe that same pattern is not specific to users, but to all the entities.

@lrozenblyum
Copy link
Author

I see @mposolda.
Main point is that for error 400 (Bad request) we don't get any details that could help identifying the cause of the problem (for the update method). However for create we do receive those details in the Response which are really helpful (maybe they were not initially supposed to be used by clients but at least for logging purposes they provide crucial information about what went wrong).

If the exception that is now thrown by the update method contained additional details (not just the status code) that would definitely solve the original issue.

@lrozenblyum lrozenblyum changed the title UserResource.update: return Response UserResource.update: provide error details Jan 21, 2025
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

No branches or pull requests

2 participants