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

Ensuring Hyrax::UserStatImporter returns false on exception #5874

Merged

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Sep 20, 2022

Prior to this commit, in the retriable block, if we hit an exception, we'd return true and upstream we'd raise an exception.

In the below code, the extract_stats_for is returning true, because calls to Rails.logger.info return true.

view_stats = extract_stats_for(object: file, from: FileViewStat, start_date: start_date, user: user)
stats = tally_results(view_stats, :views, stats) if view_stats.present?

This resulted in attempting to call .each on the view_stats; which was true.

This builds on a reported issue in Slack.

Closes #5873

@samvera/hyrax-code-reviewers

This can be back-ported to 3.4.x.

Prior to this commit, in the retriable block, if we hit an exception,
we'd return `true` and upstream we'd raise an exception.

In the below code, the `extract_stats_for` is returning true, because
calls to `Rails.logger.info` return true.

```ruby
view_stats = extract_stats_for(object: file, from: FileViewStat, start_date: start_date, user: user)
stats = tally_results(view_stats, :views, stats) if view_stats.present?
```

This resulted in attempting to call `.each` on the `view_stats`; which
was true.

This builds on a [reported issue in Slack][1].

Closes #5873

[1]:https://samvera.slack.com/archives/C0F9JQJDQ/p1663608069939129
@dlpierce dlpierce added the notes-bugfix Release Notes: Fixed a bug label Sep 20, 2022
@dlpierce dlpierce merged commit 2ea884f into main Sep 20, 2022
@dlpierce dlpierce deleted the jeremyf-ensuring-stat-importer-handles-logged-exceptions branch September 20, 2022 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport needed notes-bugfix Release Notes: Fixed a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hyrax::UserStatImporter raises exception when a retry raises exception
3 participants