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

Title logging #32585

Merged
merged 3 commits into from Jan 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 12 additions & 7 deletions dashboard/app/controllers/activities_controller.rb
@@ -1,5 +1,6 @@
require 'cdo/activity_constants'
require 'cdo/share_filtering'
require 'cdo/firehose'

class ActivitiesController < ApplicationController
include LevelsHelper
Expand Down Expand Up @@ -52,9 +53,9 @@ def milestone
if @level.game.sharing_filtered?
begin
share_failure = ShareFiltering.find_share_failure(params[:program], locale)
rescue OpenURI::HTTPError, IO::EAGAINWaitReadable => share_checking_error
rescue OpenURI::HTTPError, IO::EAGAINWaitReadable => share_filtering_error
# If WebPurify or Geocoder fail, the program will be allowed, and we
# retain the share_checking_error to log it alongside the level_source
# retain the share_filtering_error to log it alongside the level_source
# ID below.
end
end
Expand All @@ -64,11 +65,15 @@ def milestone
@level,
params[:program].strip_utf8mb4
)
if share_checking_error
slog(
tag: 'share_checking_error',
error: "#{share_checking_error.class.name}: #{share_checking_error}",
level_source_id: @level_source.id
if share_filtering_error
FirehoseClient.instance.put_record(
study: 'share_filtering',
study_group: 'v0',
event: 'share_filtering_error',
data_string: "#{share_filtering_error.class.name}: #{share_filtering_error}",
data_json: {
level_source_id: @level_source.id
}.to_json
)
end
end
Expand Down
28 changes: 20 additions & 8 deletions dashboard/test/controllers/activities_controller_test.rb
Expand Up @@ -5,7 +5,15 @@ class ActivitiesControllerTest < ActionController::TestCase
include LevelsHelper
include UsersHelper

def stub_firehose
FirehoseClient.instance.stubs(:put_record).with do |args|
@firehose_record = args
true
end
end

setup do
stub_firehose
client_state.reset
Gatekeeper.clear

Expand Down Expand Up @@ -899,13 +907,10 @@ def expect_controller_logs_milestone_regexp(regexp)
assert_equal_expected_keys expected_response, JSON.parse(@response.body)
end

test 'sharing program with http error slogs' do
test 'sharing program with http error logs' do
# allow sharing when there's an error, slog so it's possible to look up and review later

WebPurify.stubs(:find_potential_profanity).raises(OpenURI::HTTPError.new('something broke', 'fake io'))
@controller.expects(:slog).with(:tag, :error, :level_source_id) do |params|
params[:tag] == 'share_checking_error' && params[:error] == 'OpenURI::HTTPError: something broke' && !params[:level_source_id].nil?
end
@controller.expects(:slog).with(:tag) {|params| params[:tag] == 'activity_finish'}

assert_creates(LevelSource) do
Expand All @@ -917,15 +922,17 @@ def expect_controller_logs_milestone_regexp(regexp)
end

assert_response :success

assert @firehose_record[:study], 'share_filtering'
assert @firehose_record[:event], 'share_filtering_error'
assert @firehose_record[:data_string], 'OpenURI::HTTPError: something broke'
refute_nil @firehose_record[:data_json]["level_source_id"]
end

test 'sharing program with IO::EAGAINWaitReadable error slogs' do
test 'sharing program with IO::EAGAINWaitReadable error logs' do
WebPurify.stubs(:find_potential_profanity).raises(IO::EAGAINWaitReadable)
# allow sharing when there's an error, slog so it's possible to look up and review later

@controller.expects(:slog).with(:tag, :error, :level_source_id) do |params|
params[:tag] == 'share_checking_error' && params[:error] == 'IO::EAGAINWaitReadable: Resource temporarily unavailable' && !params[:level_source_id].nil?
end
@controller.expects(:slog).with(:tag) {|params| params[:tag] == 'activity_finish'}

assert_creates(LevelSource) do
Expand All @@ -937,6 +944,11 @@ def expect_controller_logs_milestone_regexp(regexp)
end

assert_response :success

assert @firehose_record[:study], 'share_filtering'
assert @firehose_record[:event], 'share_filtering_error'
assert @firehose_record[:data_string], 'IO::EAGAINWaitReadable: Resource temporarily unavailable'
refute_nil @firehose_record[:data_json]["level_source_id"]
end

test 'sharing program with swear word in Spanish rejects word' do
Expand Down
15 changes: 13 additions & 2 deletions shared/middleware/helpers/profanity_privacy_helper.rb
@@ -1,6 +1,7 @@
require 'cdo/share_filtering'
require_relative './bucket_helper.rb'
require_relative './source_bucket.rb'
require 'cdo/firehose'

class ProfanityPrivacyError < StandardError
def initialize(flagged_text)
Expand Down Expand Up @@ -30,8 +31,18 @@ def channel_policy_violation?(channel_id)
def title_profanity_privacy_violation(name, locale)
share_failure = begin
ShareFiltering.find_name_failure(name, locale)
rescue OpenURI::HTTPError, IO::EAGAINWaitReadable
# If WebPurify or Geocoder are unavailable, default to viewable
rescue OpenURI::HTTPError, IO::EAGAINWaitReadable => error
# If WebPurify or Geocoder are unavailable, default to viewable, but log error
FirehoseClient.instance.put_record(
study: 'share_filtering',
study_group: 'v0',
event: 'share_filtering_error',
data_string: "#{error.class.name}: #{error}",
data_json: {
name: name,
locale: locale
}.to_json
)
return false
end
share_failure
Expand Down