search

nishio-dens / bitbucket-pullrequest-builder-plugin

Bitbucket Pull Request Builder Plugin for Jenkins
Other
125 stars 145 forks source link

Return our response body as a string #183

Closed CodeMonk closed 5 years ago

CodeMonk commented 5 years ago

While also more efficient, I think it may solve the socket timeout problem, where read() hangs in the middle of getResponseBodyAsStream()

CodeMonk commented 5 years ago

This may fix #180 by letting the http library do the read with the socket timeout, instead of doing a read, which may ignore the exception.

CodeMonk commented 5 years ago

@chrisLeeTW Would you mind reviewing this?

I have low confidence in my fix, but, given the stack traces in https://issues.jenkins-ci.org/browse/JENKINS-37632 and the information in #180, this certainly looks like a hung socket.

However, the entire fix is dependant on the assumption that: IOUtils.readString() can sometimes ignore the socket exception, and that req.getResponseBodyAsString() will not ignore any socket timeouts.

I have very low confidence in the fix, since I do not know if the above paragraph is accurate.

Thanks in advance,

-Dave

chrisLeeTW commented 5 years ago

@CodeMonk In my previous pull request (https://github.com/nishio-dens/bitbucket-pullrequest-builder-plugin/pull/159), I already change req.getResponseBodyAsStream() to req.getResponseBodyAsString().

And issue #162 is happend after this pull request merged.

So I think this pull request may not fix issue #162 in my opinion.

CodeMonk commented 5 years ago

@chrisLeeTW Fair enough - I'm going to kill this one, and merge yours then.