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

Update fields to extract AuthAPIError messages #566

Merged
merged 1 commit into from
May 26, 2022

Conversation

sirosen
Copy link
Member

@sirosen sirosen commented May 25, 2022

"message" does not appear to match the API -- it is never used. "error" is a copy of "code" in some cases, but it is only used in the
top-level error wrapper (not the nested error doc we extract). "title" is actively used as an alternative to "detail" and was not being checked. Add this to the fields.

This can be refined further over time if we see issues with this message parsing.

The change could technically be breaking for code which uses AuthAPIError.message to drive logic. e.g.

try: ...
except AuthAPIError as e:
    if "foostr" in e.message: ...
    raise

However, we're improving behavior and staying consistent with the spirit of semver. The above kind of logic presumably should have been written as

try: ...
except AuthAPIError as e:
    if "foostr" in e.raw_text: ...
    raise

"message" does not appear to match the API -- it is never used.
"error" is a copy of "code" in some cases, but it is only used in the
top-level error wrapper (not the nested error doc we extract).
"title" *is* actively used as an alternative to "detail" and was not
being checked. Add this to the fields.

This can be refined further over time if we see issues with this
message parsing.

The change could technically be breaking for code which uses
`AuthAPIError.message` to drive logic. e.g.

    try: ...
    except AuthAPIError as e:
        if "foostr" in e.message: ...
        raise

However, we're improving behavior and staying consistent with the
spirit of semver. The above kind of logic presumably _should have
been_ written as

    try: ...
    except AuthAPIError as e:
        if "foostr" in e.raw_text: ...
        raise
@sirosen sirosen merged commit 3bfcd06 into globus:main May 26, 2022
@sirosen sirosen deleted the auth-errmessage-fields branch May 26, 2022 16:09
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.

1 participant