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

Response bodies are stringified based on Content-type when they shouldn't #9

Open
patrickbkr opened this issue May 29, 2019 · 0 comments

Comments

@patrickbkr
Copy link
Contributor

During response validation the body is read and set (here). I think that's necessary to not loose the body and realize Supplies. This in turn causes the body to go through the body parser which is selected based on the Content-type. That shouldn't be.

Respective IRC snippet:

11:01:05 patrickb: I found the reason for my yesterdays problem of response bodies being stringified.
https://github.com/croservices/cro-openapi-routes-from-definition/blob/master/lib/Cro/OpenAPI/Routes...
11:02:26 patrickb: RoutesFromDefinition parses the response body to check it. The default parsers are selected based on the Content-type header. Thus the stringification.
11:03:13 jnthn: Ah, interesting. :)
11:03:21 patrickb: I think that's not a bug per se.
11:03:41 jnthn: Well, that is a bit of a workaround that really wants a peek-body or similar
So we'd not have to take it and set it back again just not to lose it
11:04:07 patrickb: jnthn: That falls apart when using supplies.
11:04:25 jnthn: What does?
11:04:49 patrickb: Supplies are read-once, right? So reading them looses that first read.
11:06:00 jnthn: Oh. Well yes, but a peek-body would observe the bytes emitted, collect them, and set the body to the blob.
So it means we just consume the supply earlier
11:06:41 patrickb: Ah. That's basically what's happening now, just not using the body parsers.
That also means that all files go through the RAM. But I'm unsure there is a way to prevent that when validating the responses.
11:07:21 jnthn: Yes, and it's precisely that we set the result back *after* the body parser that seems to be the problem here. :-)
Me either; I think you can disable response validation though
Which tbh I'd probably do in production
11:07:50 patrickb: True.
11:08:32 jnthn: It's really good to have on while in development and running tests.
11:13:31 jnthn: Please could you make an issue about this?
11:13:37 patrickb: Now that I know what's happening, I can live with it I guess.
Will do!
11:13:55 jnthn: I might see if I can fix provide the needed method ahead of the upcoming Cro release (probably end of this week or early next week)
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

1 participant