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

improved error-handling #27

Closed
wants to merge 2 commits into from
Closed

improved error-handling #27

wants to merge 2 commits into from

Conversation

akarelas
Copy link
Contributor

Before, if an error occurred during the authorization stage, the user would just keep on visiting the oauth2 provider in an endless loop. With this patch, at least in blocking mode, get_token will return ($token, $err, $state) and the user won't enter an endless loop. The programmer can now easily check $err.

If it didn't break backwards compatibility, I would have implemented something similar for the non-blocking mode.

@akarelas
Copy link
Contributor Author

I made $err to be a hashref, so that it can be expanded in the future with extra fields if needed without breaking backwards compatibility.

@akarelas
Copy link
Contributor Author

This fix can turn this complex-looking blocking route handler:

my $token = eval { $self->get_token('facebook', on_refuse => sub { die \'refused'; }); };
if (my $e = $@) {
    if (ref $e eq 'SCALAR' and $$e eq 'refused') {
        return $self->render(text => 'refused');
    } else {
        die $e;
    }
}
if ($token) {
    # ...
}

...into this:

my ($token, $err) = $self->get_token('facebook');
if ($err and $err->{error} eq 'access_denied') {
    return $self->render(text => 'refused');
}
if ($token) {
    # ...
}

@akarelas
Copy link
Contributor Author

Of course needs documentation and tests. Will do these if you tell me you want this pull request.

@akarelas
Copy link
Contributor Author

akarelas commented May 4, 2014

Pinging for attention from Marcus.

@marcusramberg
Copy link
Owner

Ok, been going a bit back and forth on this one, but I think it's an improvement, so I'll accept it if you provide tests and documentation.

@akarelas
Copy link
Contributor Author

Will do the tests & documentation for this pull request, in a few days.

@jhthorsen
Copy link
Collaborator

This is fixed in master.

See these commits:
acb0599
7527fd0
6fff3c6

@jhthorsen jhthorsen closed this Mar 1, 2015
jhthorsen pushed a commit that referenced this pull request Mar 1, 2015
    - Fix handling of error in param, #27
    - Add new helper oauth2->auth_url
    - Add new helper oauth2->get_token
    - Add new helper oauth2->providers
    - Add eventbrite and github as providers
    - Deprecate on_xxx handlers
    - Started deprecation process for get_authorize_url() and get_token();
@akarelas akarelas deleted the akarelas-improveErrorHandling branch January 15, 2016 16:51
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.

3 participants