Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

populate response body #261

Merged
merged 1 commit into from Sep 20, 2012

Conversation

Projects
None yet
3 participants
Contributor

allenarthurgay commented Sep 20, 2012

in clientbase handleexception to allow for more flexible error handling. This is a change we have been using at Daptiv and would like to get it into the base repo so that we can more easily update our fork

@mythz mythz added a commit that referenced this pull request Sep 20, 2012

@mythz mythz Merge pull request #261 from allenarthurgay/addresponsebodytoexception
populate response body
953c810

@mythz mythz merged commit 953c810 into ServiceStack:master Sep 20, 2012

Owner

mythz commented Sep 20, 2012

cool, thx!

Owner

desunit commented Sep 20, 2012

It seems that commit broke 12 tests. @mythz, take a look at build 157. Exception is the same for most of thme:

Test(s) failed. System.NullReferenceException : Object reference not set to an instance of an object.
   at ServiceStack.WebHost.IntegrationTests.Tests.CustomerServiceValidationTests.Get_empty_request_throws_validation_exception(IRestClient client) in c:\BuildAgent\work\123e7c5a6bd2910f\tests\ServiceStack.WebHost.IntegrationTests\Tests\CustomerServiceValidationTests.cs:line 102
Owner

mythz commented Sep 20, 2012

yep, will look at trying to fix the build errors tonight

Owner

mythz commented Sep 20, 2012

hmmm seems like this contains a bug trying to re-read twice from the same stream. @allenarthurgay was this tested before this pull request was issued?

Contributor

allenarthurgay commented Sep 21, 2012

Looks like an oversight on our part - been working in production but
definitely breaks a few tests. I have a fix I'll submit shortly

On Thu, Sep 20, 2012 at 2:19 PM, Demis Bellot notifications@github.comwrote:

hmmm seems like this contains a bug trying to re-read twice from the same
stream. @allenarthurgay https://github.com/allenarthurgay was this
tested before this pull request was issued?


Reply to this email directly or view it on GitHubhttps://github.com/ServiceStack/ServiceStack/pull/261#issuecomment-8746029.

Contributor

allenarthurgay commented Sep 21, 2012

Actually will have to address this a bit later tonight. I have a fix and
it's been reviewed here, but I need to add an additional test to verify our
dependency

On Thu, Sep 20, 2012 at 5:32 PM, Allen Gay allenarthurgay@gmail.com wrote:

Looks like an oversight on our part - been working in production but
definitely breaks a few tests. I have a fix I'll submit shortly

On Thu, Sep 20, 2012 at 2:19 PM, Demis Bellot notifications@github.comwrote:

hmmm seems like this contains a bug trying to re-read twice from the same
stream. @allenarthurgay https://github.com/allenarthurgay was this
tested before this pull request was issued?


Reply to this email directly or view it on GitHubhttps://github.com/ServiceStack/ServiceStack/pull/261#issuecomment-8746029.

Owner

mythz commented Sep 21, 2012

Never mind I decided to fixed this - I don't like broken builds.

Can you verify future pull-requests get tested before submitting.
thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment