CP-1851 Add ResponseFormatException #143
Conversation
evanweible-wf
force-pushed
the
improve-body-decode-error-message
branch
from
cfb316c to
8936c1c
Compare
jayudey-wf
changed the title
|
+1 once checks pass. |
| expect(exception, new isInstanceOf<ResponseFormatException>(), | ||
| reason: | ||
| 'should throw ResponseFormatException if bytes cannot be decoded'); | ||
| print(exception); |
There was a problem hiding this comment.
Jay pointed out this leftover print and the one above, I'll remove them.
evanweible-wf
force-pushed
the
improve-body-decode-error-message
branch
from
8936c1c to
6206467
Compare
|
@sebastianmalysa-wf I removed a couple of leftover prints (see outdated diff above) |
|
+1 |
evanweible-wf
force-pushed
the
improve-body-decode-error-message
branch
from
6206467 to
f8d3e2a
Compare
Current coverage is 99.51%
|
|
@trentgrover-wf @sebastianmalysa-wf rebased and squashed, looks like the coverage comment is working but the CI check is not. No idea why, but I'll figure it out separately. |
|
+1 |
|
+1 |
evanweible-wf
force-pushed
the
improve-body-decode-error-message
branch
2 times, most recently
from
f17b88e to
3724866
Compare
|
looks like there's now a couple of test failures |
|
@trentgrover-wf waiting for #152 |
evanweible-wf
force-pushed
the
improve-body-decode-error-message
branch
from
3724866 to
5e2ef32
Compare
|
Hmm, the push build is failing and it smells like the content-shell issue that cropped up again with Dart 1.17.0, but the build says it's running 1.17.1... we'll see if the re-run passes |
|
Must have been a fluke, both builds passing now. @trentgrover-wf @maxwellpeterson-wf @dustinlessard-wf @sebastianmalysa-wf |
|
+1 |
|
+1 |
evanweible-wf
force-pushed
the
improve-body-decode-error-message
branch
from
5e2ef32 to
8b45779
Compare
| expect(exception.toString(), contains('Body could not be encoded')); | ||
| expect(exception.toString(), contains('Content-Type: $contentType')); | ||
| expect(exception.toString(), contains('Encoding: ${ASCII.name}')); | ||
| expect(exception.toString(), contains('bodyçå®')); |
There was a problem hiding this comment.
Added these last 4 expectations to test the message ^
| expect(exception.toString(), contains('Bytes could not be decoded')); | ||
| expect(exception.toString(), contains('Content-Type: $contentType')); | ||
| expect(exception.toString(), contains('Encoding: ${ASCII.name}')); | ||
| expect(exception.toString(), contains(UTF8.encode('bodyçå®').toString())); |
There was a problem hiding this comment.
Added these last 4 expectations to test the message ^
|
@trentgrover-wf @maxwellpeterson-wf @dustinlessard-wf @sebastianmalysa-wf I updated the two tests to include additional asserts that exercise the Thanks @jayudey-wf for catching that! |
|
+1 |
- Improve error messaging when response body cannot be decoded/encoded
evanweible-wf
force-pushed
the
improve-body-decode-error-message
branch
from
8b45779 to
fb7bd0a
Compare
|
@trentgrover-wf @maxwellpeterson-wf @dustinlessard-wf @sebastianmalysa-wf sorry, missed a dartfmt. |
|
+1 |
|
+1 |
|
QA Resource Approval: +10
Merging into master. |
Fixes #131.
Issue
#131
Solution
When a response body cannot be properly decoded/encoded using the
Encodingdictated by thecontent-typeheader, aResponseFormatExceptionwill now be thrown with a much more descriptive message. The content-type, encoding, and body will be included.Testing
Code Review
@trentgrover-wf
@maxwellpeterson-wf
@dustinlessard-wf
@sebastianmalysa-wf
@jayudey-wf