Skip to content

Commit 4c55c3c

Browse files
Fix string validation errors (#1740)
* Fix string validation errors Just came across this in the wild openSUSE/open-build-service#17535 Apparently sometimes the validation json is formated like that. * Add additional test cases * Fix lint: add empty line after guard clause * Use regex for nested structure test assertion --------- Co-authored-by: Keegan Campbell <[email protected]>
1 parent fbcea34 commit 4c55c3c

File tree

2 files changed

+143
-0
lines changed

2 files changed

+143
-0
lines changed

lib/octokit/error.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,8 @@ def response_error_summary
191191
return nil unless data.is_a?(Hash) && !Array(data[:errors]).empty?
192192

193193
summary = +"\nError summary:\n"
194+
return summary << data[:errors] if data[:errors].is_a?(String)
195+
194196
summary << data[:errors].map do |error|
195197
if error.is_a? Hash
196198
error.map { |k, v| " #{k}: #{v}" }

spec/octokit/client_spec.rb

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,147 @@
839839
end
840840
end
841841

842+
it 'exposes errors string' do
843+
stub_get('/boom')
844+
.to_return \
845+
status: 422,
846+
headers: {
847+
content_type: 'application/json'
848+
},
849+
body: {
850+
message: 'Validation Failed',
851+
errors: 'Issue'
852+
}.to_json
853+
begin
854+
Octokit.get('/boom')
855+
rescue Octokit::UnprocessableEntity => e
856+
expect(e.message).to include('GET https://api.github.com/boom: 422 - Validation Failed')
857+
expect(e.message).to include('Issue')
858+
end
859+
end
860+
861+
it 'handles errors as a hash' do
862+
stub_get('/boom')
863+
.to_return \
864+
status: 422,
865+
headers: { content_type: 'application/json' },
866+
body: {
867+
message: 'Validation Failed',
868+
errors: { field: 'some field', issue: 'some issue' }
869+
}.to_json
870+
begin
871+
Octokit.get('/boom')
872+
rescue Octokit::UnprocessableEntity => e
873+
expect(e.message).to include('GET https://api.github.com/boom: 422 - Validation Failed')
874+
expect(e.message).to include('Error summary:')
875+
expect(e.message).to include('[:field, "some field"]')
876+
expect(e.message).to include('[:issue, "some issue"]')
877+
end
878+
end
879+
880+
it 'handles nil errors gracefully' do
881+
stub_get('/boom')
882+
.to_return \
883+
status: 422,
884+
headers: { content_type: 'application/json' },
885+
body: {
886+
message: 'Validation Failed',
887+
errors: nil
888+
}.to_json
889+
begin
890+
Octokit.get('/boom')
891+
rescue Octokit::UnprocessableEntity => e
892+
expect(e.message).to include('GET https://api.github.com/boom: 422 - Validation Failed')
893+
expect(e.message).not_to include('Error summary:')
894+
end
895+
end
896+
897+
it 'handles errors array of strings' do
898+
stub_get('/boom')
899+
.to_return \
900+
status: 422,
901+
headers: { content_type: 'application/json' },
902+
body: {
903+
message: 'Validation Failed',
904+
errors: ['Issue 1', 'Issue 2']
905+
}.to_json
906+
begin
907+
Octokit.get('/boom')
908+
rescue Octokit::UnprocessableEntity => e
909+
expect(e.message).to include('GET https://api.github.com/boom: 422 - Validation Failed')
910+
expect(e.message).to include('Error summary:')
911+
expect(e.message).to include('Issue 1')
912+
expect(e.message).to include('Issue 2')
913+
end
914+
end
915+
916+
it 'handles errors with special characters' do
917+
stub_get('/boom')
918+
.to_return \
919+
status: 422,
920+
headers: { content_type: 'application/json' },
921+
body: {
922+
message: 'Validation Failed',
923+
errors: 'Error with <special> characters & symbols'
924+
}.to_json
925+
begin
926+
Octokit.get('/boom')
927+
rescue Octokit::UnprocessableEntity => e
928+
expect(e.message).to include('GET https://api.github.com/boom: 422 - Validation Failed')
929+
expect(e.message).to include('Error summary:')
930+
expect(e.message).to include('Error with <special> characters & symbols')
931+
end
932+
end
933+
934+
it 'handles nested structures in errors' do
935+
stub_get('/boom')
936+
.to_return \
937+
status: 422,
938+
headers: { content_type: 'application/json' },
939+
body: {
940+
message: 'Validation Failed',
941+
errors: [
942+
{ field: 'some field', issue: 'some issue' },
943+
{ details: { subfield: 'value' } }
944+
]
945+
}.to_json
946+
begin
947+
Octokit.get('/boom')
948+
rescue Octokit::UnprocessableEntity => e
949+
expect(e.message).to include('GET https://api.github.com/boom: 422 - Validation Failed')
950+
expect(e.message).to include('Error summary:')
951+
expect(e.message).to include('field: some field')
952+
expect(e.message).to include('issue: some issue')
953+
# Ruby, depending on the version, may format the hash differently
954+
# either with a ligature (=>) or a colon (:)
955+
# '{:subfield => "value"}' or '{subfield: "value"}'
956+
# so we use a regex to match either for the test assertion
957+
expect(e.message).to match(
958+
/details: \{(?::subfield\s*=>\s*"value"|subfield:\s*"value")\}/
959+
)
960+
end
961+
end
962+
963+
it 'handles mixed-type errors array' do
964+
stub_get('/boom')
965+
.to_return \
966+
status: 422,
967+
headers: { content_type: 'application/json' },
968+
body: {
969+
message: 'Validation Failed',
970+
errors: ['Issue', { field: 'some field', issue: 'some issue' }]
971+
}.to_json
972+
begin
973+
Octokit.get('/boom')
974+
rescue Octokit::UnprocessableEntity => e
975+
expect(e.message).to include('GET https://api.github.com/boom: 422 - Validation Failed')
976+
expect(e.message).to include('Error summary:')
977+
expect(e.message).to include('Issue')
978+
expect(e.message).to include('field: some field')
979+
expect(e.message).to include('issue: some issue')
980+
end
981+
end
982+
842983
it 'knows the difference between different kinds of forbidden' do
843984
stub_get('/some/admin/stuffs').to_return(status: 403)
844985
expect { Octokit.get('/some/admin/stuffs') }.to raise_error Octokit::Forbidden

0 commit comments

Comments
 (0)