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

NoneType object has no attribute author #7066

Merged
merged 1 commit into from
Oct 13, 2022

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Oct 9, 2022

Closes Sentry OL-WEB-88 which has been happening for 7 months and happened 17k times in the past 30 days and 1.1k times in the past 24 hours.

When get_version(page.key, page.revision) returns None, properly display the page instead of raising the exception AttributeError: 'NoneType' object has no attribute 'author'

Technical

Testing

Screenshot

Stakeholders

@mekarpeles mekarpeles self-assigned this Oct 10, 2022
@mekarpeles mekarpeles added Priority: 1 Do this week, receiving emails, time sensitive, . [managed] On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing labels Oct 10, 2022
@mekarpeles
Copy link
Member

I think even with this change there's still an issue:
testing openlibrary org_feed_debug=true

@cclauss
Copy link
Contributor Author

cclauss commented Oct 13, 2022

On the modified page openlibrary/templates/diff.html there are just three instances of author as an attribute.
Line 2 and 3 in the diff below. All three of those instances are short-circuited (not run) by the proposed change.

I cannot find the proposed change on testing dev-merged. Line two is not modified as proposed in this pull request.

➜  openlibrary git:(dev-merged) ✗ hostname
ol-dev1.us.archive.org
➜  openlibrary git:(dev-merged) ✗ git grep author openlibrary/templates/diff.html

openlibrary/templates/diff.html:    $elif a.type.key in ["/type/author"]:
openlibrary/templates/diff.html:        $if v.author:
openlibrary/templates/diff.html:            <span class="brown">$_('by') <a href="$v.author.key">$v.author.displayname</a> $datestr(v.created)</span>
openlibrary/templates/diff.html:                elif p == "authors":
openlibrary/templates/diff.html:                    label = "authors"
openlibrary/templates/diff.html:                    t = "/type/author"
openlibrary/templates/diff.html:                    ap = a.get_authors()
openlibrary/templates/diff.html:                    bp = b.get_authors()

@cclauss cclauss removed the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Oct 13, 2022
@mekarpeles
Copy link
Member

I think we may be trading errors -- I may have run this on ol-dev1 ol-mek. Let's merge and see if sentry confirms. Can't hurt?

@mekarpeles mekarpeles merged commit f0822a5 into internetarchive:master Oct 13, 2022
@cclauss cclauss deleted the None-has-no-author branch October 13, 2022 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 1 Do this week, receiving emails, time sensitive, . [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants