Skip to content

Raise a specific exception if response from API is HTTP 401#9

Merged
anero merged 2 commits into
mainfrom
handle_401_responses
Nov 17, 2025
Merged

Raise a specific exception if response from API is HTTP 401#9
anero merged 2 commits into
mainfrom
handle_401_responses

Conversation

@anero
Copy link
Copy Markdown
Contributor

@anero anero commented Nov 17, 2025

Raise an UnauthorizedError error if the service's response status code is 401.

@anero anero added the enhancement New feature or request label Nov 17, 2025
Copy link
Copy Markdown
Contributor

@lavaturtle lavaturtle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines 32 to 33
# Q: Shouldn't we also raise for 4xx errors?
# A: We let TypedErrorMiddleware handle those, in case they have useful info.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment still relevant?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I'm not sure. From the comment it seems that we thought that 4xx errors would include additional information about the error in the body. In the case of 401 errors the XML returned from the service is:

<?xml version="1.0" encoding="UTF-8"?>
<response>
  <status>401</status>
  <error>Unauthorized</error>
  <message>You have attempted to access a restricted resource</message>
</response>

So we probably only care about errors with code 400 for TypedErrorMiddleware. MobileCommons' API docs (https://faq.mobilecommons.com/hc/en-us/articles/38902578710811-Mobile-Commons-Rest-API#errors) are completely broken, so I don't know which status code is sent for each of the specific errors we've handling for on https://github.com/controlshift/upland_mobile_commons_rest/blob/main/lib/upland_mobile_commons_rest/errors.rb#L140, so"ll just update the comment to reflect this small change without making any other updates just in case.

@anero anero force-pushed the handle_401_responses branch from 6d85e44 to ac0984e Compare November 17, 2025 19:27
@anero anero merged commit 29496d2 into main Nov 17, 2025
@anero anero deleted the handle_401_responses branch November 17, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants