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.

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

Jump to bottom

Hyrax::UserStatImporter raises exception when a retry raises exception #5873

Closed
jeremyf opened this issue Sep 20, 2022 · 0 comments · Fixed by #5874
Closed

Hyrax::UserStatImporter raises exception when a retry raises exception #5873

jeremyf opened this issue Sep 20, 2022 · 0 comments · Fixed by #5874

Comments

@jeremyf
Copy link
Contributor

jeremyf commented Sep 20, 2022

Descriptive summary

In Hyrax v3.4.1 and up (and perhaps earlier) the following exception occurred:

[Hyrax:UserStatImporter lines where UserStat Load succeeds - excluded from message]
I, [2022-09-19T17:16:18.061623 #29033]  INFO -- : Hyrax::UserStatImporter: Retried FileViewStat on #<struct Struct::UserRecord id=1, user_key="hello@world.com", last_stats_update=Wed, 10 Sep 2014 00:00:00 +0000> for FileSet c247ds08x too many times.
I, [2022-09-19T17:16:18.061743 #29033]  INFO -- : Hyrax::UserStatImporter: Retried FileViewStat on #<struct Struct::UserRecord id=1, user_key="hello@world.com", last_stats_update=Wed, 10 Sep 2014 00:00:00 +0000> for FileSet c247ds08x too many times.
I, [2022-09-19T17:16:18.064223 #29033]  INFO -- : Hyrax::UserStatImporter: Last exception undefined method `hyrax__analytics__google__pageviews' for #<Legato::Management::Profile:0x000055754be41d60>
I, [2022-09-19T17:16:18.064292 #29033]  INFO -- : Hyrax::UserStatImporter: Last exception undefined method `hyrax__analytics__google__pageviews' for #<Legato::Management::Profile:0x000055754be41d60>
rake aborted!
NoMethodError: undefined method `each' for true:TrueClass
/srv/rails/app/services/hyrax/user_stat_importer.rb:119:in `tally_results'
/srv/rails/app/services/hyrax/user_stat_importer.rb:54:in `block in process_files'
/srv/rails/app/services/hyrax/user_stat_importer.rb:51:in `each'
/srv/rails/app/services/hyrax/user_stat_importer.rb:51:in `process_files'
/srv/rails/app/services/hyrax/user_stat_importer.rb:31:in `block in import'
/srv/rails/app/services/hyrax/user_stat_importer.rb:24:in `each'
/srv/rails/app/services/hyrax/user_stat_importer.rb:24:in `import'
/usr/local/bundle/gems/hyrax-3.4.1/lib/tasks/stats_tasks.rake:7:in `block (3 levels) in <top (required)>'
/usr/local/bundle/gems/rake-13.0.6/exe/rake:27:in `<top (required)>'
/usr/local/bin/bundle:23:in `load'
/usr/local/bin/bundle:23:in `<main>'
Tasks: TOP => hyrax:stats:user_stats

See Slack thread for details

Expected behavior

We should not have an each call on TrueClass objects.
jeremyf added a commit that referenced this issue Sep 20, 2022
@jeremyf
Ensuring Hyrax::UserStatImporter returns false on exception
c91e0dd 
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
@jeremyf jeremyf mentioned this issue Sep 20, 2022
Merged
jeremyf added a commit that referenced this issue Sep 20, 2022
@jeremyf
Ensuring Hyrax::UserStatImporter returns false on exception
8178c68 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ensuring Hyrax::UserStatImporter returns false on exception samvera/hyrax
1 participant
@jeremyf