http: Improve Response html()/json() error message if body is null

[daniel-shuy]

Closes #2188

When Response.body is nil, instead of displaying the error message invalid type <nil>, expected string, []byte or ArrayBuffer (which is being propagated from util.ToBytes()/util.ToString()), Response.html() displays the body is null so we can't transform it to html - this likely was because of a request error getting the response while Response.json() displays the body is null so we can't transform it to json - this likely was because of a request error getting the response.

[CLAassistant]

All committers have signed the CLA.

[codebien]

Hi @daniel-shuy,
thanks for your contribution! 🎉 👏

It would be good if we can add some test coverage for these cases.

[codebien]

I don't think we need it, in this way, we are making this call also in the positive cases that it shouldn't be necessary.

[daniel-shuy]

I see, I've reverted it

[daniel-shuy]

@codebien I've added tests as you suggested, using httpbin's /status/{codes} to simulate an empty response body

[codebien]

@daniel-shuy the tests are failing, response.body is not really null so it panics on err.Error() because err is nil.

You could test using responseType: none

http.get("HTTPBIN_URL/some", {responseType: 'none'}).html();

or re-assigning the body

var res = http.get("HTTPBIN_URL/some"); res.body = null; res.html();

[daniel-shuy]

@codebien using responseType: none worked, thanks!

I still can't figure out why golangci-lint is failing with:

paralleltest  Function TestResponse has missing the call to method parallel in the test run

[codecov-commenter]

Codecov Report

Merging #2195 (6e59713) into master (42f2e60) will increase coverage by 0.07%.
The diff coverage is 79.14%.

❗ Current head 6e59713 differs from pull request most recent head 8666d19. Consider uploading reports for the commit 8666d19 to get more accurate results

@@            Coverage Diff             @@
##           master    #2195      +/-   ##
==========================================
+ Coverage   72.71%   72.79%   +0.07%     
==========================================
  Files         184      184              
  Lines       14571    14671     +100     
==========================================
+ Hits        10596    10680      +84     
- Misses       3333     3343      +10     
- Partials      642      648       +6     

Flag	Coverage Δ
ubuntu	72.71% <79.14%> (+0.06%)	⬆️
windows	72.45% <79.14%> (+0.07%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
cmd/login_cloud.go	0.00% <0.00%> (ø)
cmd/login_influxdb.go	0.00% <0.00%> (ø)
cmd/ui.go	20.61% <0.00%> (ø)
js/compiler/compiler.go	55.00% <ø> (ø)
lib/execution_segment.go	92.73% <ø> (ø)
lib/executor/ramping_vus.go	94.66% <ø> (ø)
lib/metrics/metrics.go	100.00% <ø> (ø)
ui/form_fields.go	45.71% <0.00%> (ø)
output/influxdb/output.go	78.57% <58.62%> (-3.61%)	⬇️
js/modules/k6/grpc/client.go	77.56% <78.94%> (-0.09%)	⬇️
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66ed1ae...8666d19. Read the comment docs.

[daniel-shuy]

Adding t.Parallel() causes the tests to fail with race detected during execution of test. I've ignored it using //nolint:paralleltest

[codebien]

Can you also rebase the history squashing the fixups commits on the first commit, please? In this way, it will be easier for the next reviewer to check it. Thanks

[daniel-shuy]

Can you also rebase the history squashing the fixups commits on the first commit, please? In this way, it will be easier for the next reviewer to check it. Thanks

Sure, I usually prefer to squash after its approved so that only the changes need to be re-reviewed.

[codebien]

LGTM 🎉 let's wait @imiric's review

[imiric]

Thanks for this @daniel-shuy!

LGTM, just a suggestion to fix the casing on HTML/JSON and to concatenate the error message to avoid silencing the linter. The concatenation should be negligible and it's also more readable in the code IMO.

[daniel-shuy]

@imiric done, thanks!