Skip to content

Better messaging on 401 and 403 errors from GitHub #320

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.

Sign up for GitHub

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

Merged
asmeurer merged 6 commits into from
Sep 19, 2018
Merged

Better messaging on 401 and 403 errors from GitHub #320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b0d0bde
Don't print a traceback on KeyboardInterrupt
asmeurer Aug 27, 2018
e3dc252
Better messaging on 401 and 403 errors from GitHub
asmeurer Aug 27, 2018
40545d2
Don't print the minutes with a leading 0
asmeurer Aug 27, 2018
2696f6f
Remove unused import
asmeurer Aug 27, 2018
1ecec1e
Merge branch 'master' into 403
asmeurer Sep 19, 2018
cc7ece0
Always print the exit error messages in red
asmeurer Sep 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions doctr/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

from .local import (generate_GitHub_token, encrypt_variable, encrypt_to_file,
upload_GitHub_deploy_key, generate_ssh_key, check_repo_exists,
GitHub_login, guess_github_repo, AuthenticationFailed)
GitHub_login, guess_github_repo, AuthenticationFailed, GitHubError)
from .travis import (setup_GitHub_push, commit_docs, push_docs,
get_current_repo, sync_from_log, find_sphinx_build_dir, run,
get_travis_branch, copy_to_tmp, checkout_deploy_branch)
Expand Down Expand Up @@ -248,7 +248,9 @@ def process_args(parser):
try:
return args.func(args, parser)
except RuntimeError as e:
sys.exit("Error: " + e.args[0])
sys.exit(red("Error: " + e.args[0]))
except KeyboardInterrupt:
sys.exit(red("Interrupted by user"))

def on_travis():
return os.environ.get("TRAVIS_JOB_NUMBER", '')
Expand Down Expand Up @@ -399,6 +401,8 @@ def configure(args, parser):
is_private = check_repo_exists(build_repo, service='github', **login_kwargs)
check_repo_exists(build_repo, service='travis')
get_build_repo = True
except GitHubError:
raise
except RuntimeError as e:
print(red('\n{!s:-^{}}\n'.format(e, 70)))

Expand All @@ -413,6 +417,8 @@ def configure(args, parser):
check_repo_exists(deploy_repo, service='github', **login_kwargs)

get_deploy_repo = True
except GitHubError:
raise
except RuntimeError as e:
print(red('\n{!s:-^{}}\n'.format(e, 70)))

Expand Down
71 changes: 66 additions & 5 deletions doctr/local.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import re
from getpass import getpass
import urllib
import datetime

import requests
from requests.auth import HTTPBasicAuth
Expand Down Expand Up @@ -130,22 +131,78 @@ def GitHub_login(*, username=None, password=None, OTP=None, headers=None):
if two_factor:
if OTP:
print(red("Invalid authentication code"))
# For SMS, we have to make a fake request (that will fail without
# the OTP) to get GitHub to send it. See https://github.com/drdoctr/doctr/pull/203
auth_header = base64.urlsafe_b64encode(bytes(username + ':' + password, 'utf8')).decode()
login_kwargs = {'auth': None, 'headers': {'Authorization': 'Basic {}'.format(auth_header)}}
try:
generate_GitHub_token(**login_kwargs)
except requests.exceptions.HTTPError:
except (requests.exceptions.HTTPError, GitHubError):
pass
print("A two-factor authentication code is required:", two_factor.split(';')[1].strip())
OTP = input("Authentication code: ")
return GitHub_login(username=username, password=password, OTP=OTP, headers=headers)

raise AuthenticationFailed("invalid username or password")

r.raise_for_status()
GitHub_raise_for_status(r)
return {'auth': auth, 'headers': headers}


class GitHubError(RuntimeError):
pass

def GitHub_raise_for_status(r):
"""
Call instead of r.raise_for_status() for GitHub requests

Checks for common GitHub response issues and prints messages for them.
"""
# This will happen if the doctr session has been running too long and the
# OTP code gathered from GitHub_login has expired.

# TODO: Refactor the code to re-request the OTP without exiting.
if r.status_code == 401 and r.headers.get('X-GitHub-OTP'):
raise GitHubError("The two-factor authentication code has expired. Please run doctr configure again.")
if r.status_code == 403 and r.headers.get('X-RateLimit-Remaining') == '0':
reset = int(r.headers['X-RateLimit-Reset'])
limit = int(r.headers['X-RateLimit-Limit'])
reset_datetime = datetime.datetime.fromtimestamp(reset, datetime.timezone.utc)
relative_reset_datetime = reset_datetime - datetime.datetime.now(datetime.timezone.utc)
# Based on datetime.timedelta.__str__
mm, ss = divmod(relative_reset_datetime.seconds, 60)
hh, mm = divmod(mm, 60)
def plural(n):
return n, abs(n) != 1 and "s" or ""

s = "%d minute%s" % plural(mm)
if hh:
s = "%d hour%s, " % plural(hh) + s
if relative_reset_datetime.days:
s = ("%d day%s, " % plural(relative_reset_datetime.days)) + s
authenticated = limit >= 100
message = """\
Your GitHub API rate limit has been hit. GitHub allows {limit} {un}authenticated
requests per hour. See {documentation_url}
for more information.
""".format(limit=limit, un="" if authenticated else "un", documentation_url=r.json()["documentation_url"])
if authenticated:
message += """
Note that GitHub's API limits are shared across all oauth applications. A
common cause of hitting the rate limit is the Travis "sync account" button.
"""
else:
message += """
You can get a higher API limit by authenticating. Try running doctr configure
again without the --no-upload-key flag.
"""
message += """
Your rate limits will reset in {s}.\
""".format(s=s)
raise GitHubError(message)
r.raise_for_status()
Copy link
Contributor

@moorepants moorepants Aug 28, 2018

Choose a reason for hiding this comment

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

This all seems clear to me. Should be very helpful.



def GitHub_post(data, url, *, auth, headers):
"""
POST the data ``data`` to GitHub.
Expand All @@ -154,7 +211,7 @@ def GitHub_post(data, url, *, auth, headers):

"""
r = requests.post(url, auth=auth, headers=headers, data=json.dumps(data))
r.raise_for_status()
GitHub_raise_for_status(r)
return r.json()


Expand Down Expand Up @@ -185,7 +242,7 @@ def generate_GitHub_token(*, note="Doctr token for pushing to gh-pages from Trav
def delete_GitHub_token(token_id, *, auth, headers):
"""Delete a temporary GitHub token"""
r = requests.delete('https://api.github.com/authorizations/{id}'.format(id=token_id), auth=auth, headers=headers)
r.raise_for_status()
GitHub_raise_for_status(r)


def upload_GitHub_deploy_key(deploy_repo, ssh_key, *, read_only=False,
Expand Down Expand Up @@ -265,7 +322,11 @@ def check_repo_exists(deploy_repo, service='github', *, auth=None, headers=None)
repo=repo,
service=service))

r.raise_for_status()
if service == 'github':
GitHub_raise_for_status(r)
else:
r.raise_for_status()

private = r.json().get('private', False)

if wiki and not private:
Expand Down