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

better errors #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

obsd
Copy link
Contributor

@obsd obsd commented Feb 22, 2022

No description provided.

@obsd obsd requested a review from asafc February 22, 2022 14:32
@linear
Copy link

linear bot commented Feb 22, 2022

PER-348 Change SDKs to throw when no connection to PDP

@Oded the following changes are needed in this ticket:

  • for each SDK in [Node.js, python, C#, Java]:
    • make sure the enforcer's permit.check() method throws a PermitConnectionError when there is no connection to the PDP, that can actually be true to all api methods
    • change the instructions in the backend to include the pseudocode:
try {
    if (permitted) {
        res.status(200).send(${user.firstName} ${user.lastName} is PERMITTED to read article!);
    } else {
        res.status(403).send(${user.firstName} ${user.lastName} is NOT PERMITTED to read article!);
} catch PermitConnectionError {
        // No connection to Permit.io PDP, did you run the container?
        res.status(403).send("No connection to Permit.io PDP, did you run the container?")
}

These changes will result in 5 pull requests:

  • Node.js SDK changes
  • python SDK changes
  • Java SDK changes
  • C# SDK changes
  • backend install instruction changes (for all 4 languages)

-----

We should change the if code to support 3 cases - res.status(200), status(403), and NULL

This way, we will be able to guide the users that the connection wasn't received and keep the change locally

What Oded and myself got to:
try {
if (permitted && res && res.status(200)) {
res.status(200).send(${user.firstName} ${user.lastName} is PERMITTED to read article!);
} else if (res && res.status(403)) {
res.status(403).send(${user.firstName} ${user.lastName} is NOT PERMITTED to read article!);
}
} catch {
res.send(received nothing);
}

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