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

Internal server error when request payload cannot be encoded to UTF-8 #103

Open
lextiz opened this issue Mar 22, 2019 · 9 comments
Open

Comments

@lextiz
Copy link
Contributor

lextiz commented Mar 22, 2019

I am trying to stub a response for a PUT request that has a zip file as a payload. As far as I understand from the documentation this is a valid use case. However the stub server responds with 500 code and has the following stack trace in logs:

INFO  WEBrick 1.3.1
INFO  ruby 2.2.2 (2015-04-13) [x86_64-linux]
INFO  WEBrick::HTTPServer#start: pid=1 port=80
I, [2019-03-22T10:24:24.911391 #1]  INFO -- : Received request PUT /data/478ef4f8-fe03-4b91-9b3a-41164ce23fde/dataset.zip
E, [2019-03-22T10:24:24.923967 #1] ERROR -- : Error ocurred in mock service: Encoding::UndefinedConversionError - "\xEB" from ASCII-8BIT to UTF-8
E, [2019-03-22T10:24:24.924102 #1] ERROR -- : /pact/lib/vendor/ruby/2.2.0/gems/pact-mock_service-2.12.0/lib/pact/mock_service/request_handlers/interaction_replay.rb:16:in `encode'
/pact/lib/vendor/ruby/2.2.0/gems/pact-mock_service-2.12.0/lib/pact/mock_service/request_handlers/interaction_replay.rb:16:in `to_json'
/pact/lib/vendor/ruby/2.2.0/gems/pact-mock_service-2.12.0/lib/pact/mock_service/request_handlers/interaction_replay.rb:16:in `pretty_generate'
/pact/lib/vendor/ruby/2.2.0/gems/pact-mock_service-2.12.0/lib/pact/mock_service/request_handlers/interaction_replay.rb:49:in `find_response'
/pact/lib/vendor/ruby/2.2.0/gems/pact-mock_service-2.12.0/lib/pact/mock_service/request_handlers/interaction_replay.rb:41:in `respond'
/pact/lib/vendor/ruby/2.2.0/gems/pact-mock_service-2.12.0/lib/pact/mock_service/request_handlers/base_request_handler.rb:17:in `call'
/pact/lib/vendor/ruby/2.2.0/gems/rack-2.0.6/lib/rack/cascade.rb:33:in `block in call'
/pact/lib/vendor/ruby/2.2.0/gems/rack-2.0.6/lib/rack/cascade.rb:24:in `each'
/pact/lib/vendor/ruby/2.2.0/gems/rack-2.0.6/lib/rack/cascade.rb:24:in `call'
/pact/lib/vendor/ruby/2.2.0/gems/pact-mock_service-2.12.0/lib/pact/consumer/mock_service/cors_origin_header_middleware.rb:11:in `call'
/pact/lib/vendor/ruby/2.2.0/gems/pact-mock_service-2.12.0/lib/pact/consumer/mock_service/error_handler.rb:13:in `call'
/pact/lib/vendor/ruby/2.2.0/gems/pact-mock_service-2.12.0/lib/pact/mock_service/app.rb:33:in `call'
/pact/lib/vendor/ruby/2.2.0/gems/pact-mock_service-2.12.0/lib/pact/consumer/mock_service/set_location.rb:14:in `call'
/pact/lib/vendor/ruby/2.2.0/gems/rack-2.0.6/lib/rack/handler/webrick.rb:86:in `service'
/pact/lib/vendor/ruby/2.2.0/gems/webrick-1.3.1/lib/webrick/httpserver.rb:138:in `service'
/pact/lib/vendor/ruby/2.2.0/gems/webrick-1.3.1/lib/webrick/httpserver.rb:94:in `run'
/pact/lib/vendor/ruby/2.2.0/gems/webrick-1.3.1/lib/webrick/server.rb:191:in `block in start_thread'

Am I missing something or the stub server expects JSON payload for all interactions?

The assumption is wrong: the JSON object is Pact internal, but includes the actual payload.

The root cause of this specific failure is that encode method fails to encode some payload as UTF-8, is that is not meant to be interpreted as such.
In my case no validations for body are needed, is that possible to bypass the failing code by having a different contract?

No, it is used everywhere.

@lextiz
Copy link
Contributor Author

lextiz commented Mar 22, 2019

Fix proposal: wrap JSON parsing in a try/catch block, when parsing would fail than the object that is printed for debugging purposes would not be pretty printed, but the transaction would not fail. Would such a fix be accepted as PR?

@bethesque
Copy link
Member

Hm... I'm really not sure how a zip file would work. How would it get written into the JSON file?

@lextiz
Copy link
Contributor Author

lextiz commented Mar 25, 2019

What I am trying to do is to use pact stub server to return a response to a POST request, that has binary body. The attempt to encode a go object (that contains also the binary payload) to JSON and back comes from pretty_generate function, that is used only to beautify the debugging printouts. So the proposal is to wrap this logic in a try/catch block, so the transaction would not fail when pretty printing of the debug logs fails.

lextiz added a commit to lextiz/pact-mock_service that referenced this issue Mar 26, 2019
Do not pretty print a JSON object when serialization or serialization fail, for details see pact-foundation#103.
@lextiz lextiz changed the title Internal server error when passing non JSON body Internal server error when request payload cannot be encoded to UTF-8 Mar 27, 2019
@bethesque
Copy link
Member

I still don't think this is going to work. How do you expect the binary data to be encoded in the UTF-8 encoded json pact file?

@lextiz
Copy link
Contributor Author

lextiz commented Apr 3, 2019

That works for me fine, because this data is not written to a JSON file. I am not running any assertions on the binary data (except that it exists), I just want pact-mock_service not to crash when it receives a request with non-UTF-8-encodable data.
I do not think that running assertions on the binary data, that would require writing it to a JSON file is a good idea in general.

@YOU54F
Copy link
Member

YOU54F commented Apr 29, 2019

I've just come across this issue when creating a pact-test that involves uploading a pact file with a binary upload.

I found it also erroring at the same point as you, although I have had to use rescue Encoding::UndefinedConversionError

			begin
				JSON.pretty_generate(JSON.parse(object.to_json))
			  rescue JSON::ParserError
				object
			  rescue Encoding::UndefinedConversionError
				object
			  end

I am not sure if pact is going to work for my current use case (mutlipart/form-data) but spiking it today

@bethesque
Copy link
Member

Released in 3.1.1

@YOU54F
Copy link
Member

YOU54F commented Jun 9, 2019

@lextiz does this solve your issue now? I need to try it out at work tomorrow

@lextiz
Copy link
Contributor Author

lextiz commented Jun 11, 2019

@YOU54F I have tested the fix "in-place" only by replacing the ruby code in live environment and checking the API behavior, without releasing the whole library. The 3.1.1 version was not tested by me yet.

themichaelyan pushed a commit to animoto/pact-mock_service that referenced this issue Feb 4, 2022
* feat: allow mock service host to be configured

pact-foundation/pact-ruby#186

* chore(release): version 3.0.0

* fix: add missing host argument to server spawn

fixes: pact-foundation#102

* chore(release): version 3.0.1

* Improve logging robustness

Do not pretty print a JSON object when serialization or serialization fail, for details see pact-foundation#103.

* Update interaction_replay.rb

* Initial version of the unit test

* Move the test to appropriate context

* Update interaction_replay_spec.rb

* Update interaction_replay_spec.rb

* Update interaction_replay_spec.rb

* Update interaction_replay_spec.rb

* Update interaction_replay.rb

* Update interaction_replay_spec.rb

* Update interaction_replay_spec.rb

* feat: pact-stub-service log level cli opt

* chore(release): version 3.1.0

* chore(release): version 3.1.1

* feat(skip writing to pact): Use writable_interactions when writing to pact file

* Add test for #as_json

* Add feature spec

* Bump pact-support dependency

* Fix tests

* chore(release): version 3.2.0

* CI: Add 2.7 to travis ci

* fix: remove apparently unused require for thwait

* chore(release): version 3.2.1

* feat: log a warning when too many interactions are set on the mock service at once

* chore(release): version 3.3.0

* fix: put metadata on the correct decorator

* chore(release): version 3.3.1

* chore: remove jruby until somebody shows they're actually using it

* chore(release): version 3.4.0

* feat: add token, username and password options to stub service (pact-foundation#118)

Co-authored-by: Beth Skurrie <[email protected]>

* chore(release): version 3.5.0

* docs: document that you can set the broker token via an env var

[ci-skip]

* chore: separate test and deploy stages

* chore: update .travis.yml

[ci-skip]

* chore: update .travis.yml

* test: add expectations to make sure metadata: nil isn't stored in the pact (pact-foundation#119)

* feat: add 'Access-Control-Allow-Headers' = true to cors response headers (pact-foundation#121)

* chore(release): version 3.6.0

* fix:  fix Ruby 2.7 kwargs warning (pact-foundation#122)

* chore(release): version 3.6.1

* fix: update thor dependency (pact-foundation#124)

Co-authored-by: Lindsey Hattamer <[email protected]>

Co-authored-by: Lindsey Hattamer <[email protected]>

* chore: add github workflow for gem release

* chore: disable tests as they're not running properly on github workflow

* chore(release): version 3.6.2

* feat: do not require files until command is executing

* chore: update release workflow [ci-skip]

* feat: use Pact::Query.parse_string to parse query string

* chore(deps): update rake

* docs: update travis badge

* chore(release): version 3.7.0

* feat: include interaction diffs in verification response

cc: @TimothyJones

* chore: add tests to github actions

* chore: allow workflow dispatch to release

* chore(release): version 3.8.0

* chore: tests are hanging. try a subset

* chore: try rspec

* chore: disabling tests, they hang

* chore: create issue template

* feat: pass host into WEBrick options to allow configuration (pact-foundation#128)

Co-authored-by: Matthew Hall <[email protected]>

* chore(release): version 3.9.0

* chore: handle http 2

* fix: check for nil body rather than falsey body when determining how to render mocked response
Fixes: pact-foundation#99

* chore(release): version 3.9.1

Co-authored-by: Beth Skurrie <[email protected]>
Co-authored-by: Alexander Bolshakov <[email protected]>
Co-authored-by: YOU54F <[email protected]>
Co-authored-by: Beth Skurrie <[email protected]>
Co-authored-by: Simon Nizov <[email protected]>
Co-authored-by: Chavez <[email protected]>
Co-authored-by: Chavez <[email protected]>
Co-authored-by: Matt Fellows <[email protected]>
Co-authored-by: vandemark <[email protected]>
Co-authored-by: Bartek Bułat <[email protected]>
Co-authored-by: Michael R. Fleet <[email protected]>
Co-authored-by: Lindsey Hattamer <[email protected]>
Co-authored-by: mhall58 <[email protected]>
Co-authored-by: Matthew Hall <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants