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

fail calls to openlibrary.php gracefully #9977

Merged
merged 2 commits into from
Nov 2, 2024
Merged

Conversation

mekarpeles
Copy link
Member

Categorically fixes IA_Lending_API issue where Open Library breaks when archive.org/services/openlibrary.php not reachable

Was discovered when trying to hit https://openlibrary.org/account/loans. Because archive.org/services/openlibrary.php is currently blocked by mod_security WAF, the API was hitting non-json 403 response and dying.

Thie code path here was: get_waitinglist_for_user in /openlibrary/openlibrary/core/waitinglist.py which died while trying to call waitlist.extend(WaitingLoan.query(userid=account.itemname)). The WaitingLoan.query uses core.lending.IA_Lending_API which has a _post method https://github.com/internetarchive/openlibrary/compare/master...php-graceful-failure?quick_pull=1#diff-896c18b3ccfadfa425c7e24a86226343314fce7957d14a8023d3e159156b224bL1092 which has been patched to return {} instead of raise when a failure occurs.

@@ -1103,9 +1103,9 @@ def _post(self, **payload):
).json()
logger.info("POST response: %s", jsontext)
return jsontext
except Exception: # TODO: Narrow exception scope
except (JSONDecodeError, Exception): # TODO: Narrow exception scope
Copy link
Member Author

Choose a reason for hiding this comment

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

We should split this into just JSONDecodeError and/or 403... And any other Exception and continue to raise

Copy link
Member Author

Choose a reason for hiding this comment

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

We might also consider using timeout=1 or something to reduce the wait time for calls we know should be fast.

Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

I think it might be more performant if we just return {} without making the request at all until it's back up.

Otherwise +1 to the longer term fix of properly handling JSONDecodeError + 403s without having this raise and break everything!

@cdrini
Copy link
Collaborator

cdrini commented Nov 2, 2024

Mek tested, looks ok for now.

@cdrini cdrini merged commit e953316 into master Nov 2, 2024
6 checks passed
@cdrini cdrini deleted the php-graceful-failure branch November 2, 2024 00:03
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