Skip to content

1680: Improve error when exporting through an account_link #1846

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+55 −10
Open

1680: Improve error when exporting through an account_link #1846

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0f59bfc
1680: Improve error when exporting through an account_link
kkoehn Jan 28, 2025
4b5dc99
1680: add sanitation of error-message from potentially external accou…
kkoehn Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions app/services/task_service/push_external.rb
View file
Original file line number Diff line number Diff line change
@@ -9,17 +9,24 @@ def initialize(zip:, account_link:)
end

def execute
body = @zip.string
begin
response = connection.post {|request| request_parameters(request, body) }
response.success? ? nil : response.body
rescue StandardError => e
e
end
response = connection.post {|request| request_parameters(request, @zip.string) }
return nil if response.success?
return I18n.t('tasks.export_external_confirm.not_authorized', account_link: @account_link.name) if response.status == 401

handle_error(message: response.body)
rescue Faraday::ServerError => e
handle_error(error: e, message: I18n.t('tasks.export_external_confirm.server_error', account_link: @account_link.name))
rescue StandardError => e
handle_error(error: e)
end

private

def handle_error(message: nil, error: nil)
Sentry.capture_exception(error) if error.present?
ERB::Util.html_escape(message || error.to_s)
end

def request_parameters(request, body)
request.tap do |req|
req.headers['Content-Type'] = 'application/zip'
@@ -31,6 +38,9 @@ def request_parameters(request, body)

def connection
Faraday.new(url: @account_link.push_url) do |faraday|
faraday.options[:open_timeout] = 5
faraday.options[:timeout] = 5

faraday.adapter Faraday.default_adapter
end
end
4 changes: 3 additions & 1 deletion config/locales/de/controllers/tasks.yml
View file
Original file line number Diff line number Diff line change
@@ -7,7 +7,9 @@ de:
duplicate:
error_alert: Die Aufgabe konnte nicht dupliziert werden.
export_external_confirm:
error: 'Der Export der Aufgabe (%{title}) ist fehlgeschlagen. <br> Fehler: %{error}'
error: 'Der Export der Aufgabe (%{title}) ist fehlgeschlagen. <br><br> Fehler: %{error}'
not_authorized: Die Autorisierung mit "%{account_link}" konnte nicht hergestellt werden. Ist der API-Schlüssel korrekt?
server_error: Verbindung zu %{account_link} fehlgeschlagen. Gegenseite nicht erreichbar.
success: Aufgabe (%{title}) erfolgreich exportiert.
import:
internal_error: Beim Import dieser Aufgabe ist auf CodeHarbor ein interner Fehler aufgetreten.
4 changes: 3 additions & 1 deletion config/locales/en/controllers/tasks.yml
View file
Original file line number Diff line number Diff line change
@@ -7,7 +7,9 @@ en:
duplicate:
error_alert: Task could not be duplicated
export_external_confirm:
error: 'Export of task (%{title}) failed. <br> Error: %{error}'
error: 'Export of task (%{title}) failed. <br><br> Error: %{error}'
not_authorized: Authorization with could not be established with "%{account_link}". Is the API Key correct?
server_error: Connection to %{account_link} failed. Remote host unreachable.
success: Task (%{title}) successfully exported.
import:
internal_error: An internal error occurred on CodeHarbor while importing the exercise.
33 changes: 32 additions & 1 deletion spec/services/task_service/push_external_spec.rb
View file
Original file line number Diff line number Diff line change
@@ -51,7 +51,38 @@
let(:status) { 500 }
let(:response) { 'an error occured' }

it { is_expected.to be response }
it { is_expected.to eql response }

context 'when response contains problematic characters' do
let(:response) { 'an <error> occurred' }

it { is_expected.to eql 'an &lt;error&gt; occurred' }
end
end

context 'when response status is 401' do
let(:status) { 401 }
let(:response) { I18n.t('tasks.export_external_confirm.not_authorized', account_link: account_link.name) }

it { is_expected.to eq response }
end

context 'when faraday throws an error' do
let(:connection) { instance_double(Faraday::Connection) }
let(:error) { Faraday::ServerError }

before do
allow(Faraday).to receive(:new).and_return(connection)
allow(connection).to receive(:post).and_raise(error)
end

it { is_expected.to eql I18n.t('tasks.export_external_confirm.server_error', account_link: account_link.name) }

context 'when another error occurs' do
let(:error) { 'another error' }

it { is_expected.to eql 'another error' }
end
end
end

Unchanged files with check annotations Beta

View file
spec/services/proforma_service/import_task_spec.rb
expect(import_proforma_task).to eql task
end
it 'creates a predecessor for task', pending: 'task relations are currently not available' do

Check warning on line 56 in spec/services/proforma_service/import_task_spec.rb

GitHub Actions / test 

ProformaService::ImportTask#execute when task with same uuid exists in db creates a predecessor for task

Failure/Error: expect { import_proforma_task }.to change { task.reload.predecessor }.from(nil).to(be_an(Task))

NoMethodError:
  undefined method 'predecessor' for an instance of Task
expect { import_proforma_task }.to change { task.reload.predecessor }.from(nil).to(be_an(Task))
end
context 'when user is an author of task', pending: 'tasks currently have one author only' do
before { task.authors << user }
it 'creates a new task' do

Check warning on line 74 in spec/services/proforma_service/import_task_spec.rb

GitHub Actions / test 

ProformaService::ImportTask#execute when task with same uuid exists in db when user does not own task when user is an author of task creates a new task

Failure/Error: before { task.authors << user }

NoMethodError:
  undefined method 'authors' for an instance of Task
expect(import_proforma_task).to eql task
end
it 'changes existing task' do

Check warning on line 78 in spec/services/proforma_service/import_task_spec.rb

GitHub Actions / test 

ProformaService::ImportTask#execute when task with same uuid exists in db when user does not own task when user is an author of task changes existing task

Failure/Error: before { task.authors << user }

NoMethodError:
  undefined method 'authors' for an instance of Task
expect { import_proforma_task }.to change(Task, :count).by(1)
end
it 'creates a predecessor for task' do

Check warning on line 82 in spec/services/proforma_service/import_task_spec.rb

GitHub Actions / test 

ProformaService::ImportTask#execute when task with same uuid exists in db when user does not own task when user is an author of task creates a predecessor for task

Failure/Error: before { task.authors << user }

NoMethodError:
  undefined method 'authors' for an instance of Task
expect { import_proforma_task }.to change { task.reload.predecessor }.from(nil).to(be_an(Task))
end
end