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

Add UnsupportedUserLocationException for better error handling #60

Merged
merged 2 commits into from
Feb 14, 2024

Conversation

rlazo
Copy link
Collaborator

@rlazo rlazo commented Feb 14, 2024

Instead of relying on the generic message to notify users that they are trying to access the API from an unsupported location, this PR adds an specific new exception that app developers can use.

Although the underlying mechanism is still string matching, abstracting this into the SDK rather than forcing every developer to do it on their own is still a win.

Instead of relying on the generic message to notify users that they
are trying to access the API from an unsupported location, this PR
adds an specific new exception that app developers can use.

Although the underlying mechanism is still string matching,
abstracting this into the SDK rather than forcing every developer to
do it on their own is still a win.
@rlazo rlazo requested a review from daymxn February 14, 2024 17:52
@rlazo rlazo enabled auto-merge (squash) February 14, 2024 18:12
@daymxn daymxn disabled auto-merge February 14, 2024 18:57
Copy link
Collaborator

@daymxn daymxn left a comment

Choose a reason for hiding this comment

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

LGTM, but needs to be moved away from main

@rlazo rlazo changed the base branch from main to next-dev February 14, 2024 19:56
@rlazo rlazo merged commit 5f2c815 into next-dev Feb 14, 2024
5 checks passed
@rlazo rlazo deleted the rl.unsupported.location.exception branch February 14, 2024 19:56
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