I agree that this is a brittle API and it's one that has bitten us
twice (partly because - I beleive - the dev_appserver.py development
server is broken in how it deals with the cases here. This is why I
filed an issue - though not with great hope of a fix other than maybe
improving the
So GetBody just fails… It returns NoBody in this case.. which means calling
code will just break (when the original request is not one of the known types).
So, according to the referenced issue, 307/308 redirects won’t work when the
underlying request is not a known type.
This is a very
Keeping a zeroed state is important for the GetBody func because the
io.ReadCloser returned by GetBody can be read and closed. If there is
no zero state for the next time GetBody is called, it is not
idempotent. This would break the whole point of it existing.
See
I agree with you on the correct solution - vs. the OP’s request of the
GetWrapped method.
I guess I still don’t understand the “zeroed” concern though. If you adhere to
the “don’t mutate values…” then why do the zero copy at all ? The state of the
body should still be the state it was
Their is an assumption in the code that users don't mutate values under
the feet of routines that have been called. The copy does not protect
against that and is not designed for that purpose; it is there to make
the GetBody function return a zeroed copy of the body (for some
definition of zeroed
And to come full circle, this poorly declared method, with hidden internal
implementation details, is exactly the cause of OP’s initial problem. Not
expecting that the Body passed in, and retrieved later could be used as a Body
of a new request - why wouldn’t he think that ?
> On Feb 7, 2019,
I see the documented use of the types in NewRequest - you are correct - I was
wrong.
But, it could of easily also declared that if the provided Reader is also a
Lener, it uses it to determine the content length. Why have this behavior for
Closer and not for Lener? Then you don’t need the type
I didn't mention the word internal, nor did I imply it; with
documentation stating that it would be used, it is clearly *not*
internal.
If you look at the code in question, you can see a probable reason why
a Lener interface is not used; for each of the blessed types, a
concrete copy of the
I am not following. You stated that the usage of Len was internal and a type
switch on known concrete types, so how is related to how the OP was attempting
to have things work?
There is no “generally accepted use of Len()”, otherwise it would not need to
perform a type switch on known concrete
Yeah, I'm not agreeing with you.
On Thu, 2019-02-07 at 07:07 -0600, Robert Engels wrote:
> You are agreeing with me. A type switch on concrete types (that you
> control) is far different than using an available Len() method and
> assuming the same semantics.
>
> >
> > On Feb 7, 2019, at 1:05
You are agreeing with me. A type switch on concrete types (that you control) is
far different than using an available Len() method and assuming the same
semantics.
> On Feb 7, 2019, at 1:05 AM, Dan Kortschak wrote:
>
> Addressing the first sentence, it was a direct answer to a comment you
>
The reason it checks for known implementations is because it “knows the
implementation”, so it can be sure it has the correct semantics.
This is the problem with Go “duck typing”.
The real solution would be to declare a new interface called BodyContent, that
had methods of io.Reader, and a
On Wed, Feb 6, 2019 at 2:50 PM robert engels wrote:
>
> I am not sure what that has to do with the discussion. My point was that for
> it to be applicable here, it needs to be defined as part of io.Reader, since
> that is what Body is declared as. It is not, so using in the manner outlined
>
I am not sure what that has to do with the discussion. My point was that for it
to be applicable here, it needs to be defined as part of io.Reader, since that
is what Body is declared as. It is not, so using in the manner outlined is not
correct IMO.
> On Feb 6, 2019, at 3:37 PM, Dan Kortschak
The generalised semantics of Len are that it returns the number of
available elements in the collection, being a cognate of the len built-
in. This means that as you consume elements from a buffer, the Len
value reduces. This is directly equivalent to
for len(buf) != 0 {
println(buf[0])
There is no Len() defined for io.Reader. That is the crux of the issue. You are
defining it because you need it. It needs to be defined and documented by the
api specification, otherwise it is an incorrect assumption and usage - aka
brittle code.
> On Feb 6, 2019, at 9:05 AM, Burak Serdar
On Wed, Feb 6, 2019 at 7:56 AM Robert Engels wrote:
>
> But is it really? If you read the description for Len() on bytes.Buffer it is
> the length of unread portion. But that doesn’t mean the buffer isn’t just a
> portion of the entire body - it can be a chunk which is continually reloaded.
But is it really? If you read the description for Len() on bytes.Buffer it is
the length of unread portion. But that doesn’t mean the buffer isn’t just a
portion of the entire body - it can be a chunk which is continually reloaded.
This is the danger in using private APIs publically based upon
On Wed, Feb 6, 2019 at 5:15 AM Robert Engels wrote:
>
> I see now, but if that can be the case, shouldn’t the Body be documented that
> the Reader may be a ReaderWithLen, and the consumer is free to type
> check/cast? If not, you are using internal details that you should not be.
Yes, the
I see now, but if that can be the case, shouldn’t the Body be documented that
the Reader may be a ReaderWithLen, and the consumer is free to type check/cast?
If not, you are using internal details that you should not be.
This is a problem with Go in general. Because the returned object
Make sense, thanks for explanation
Il giorno mercoledì 6 febbraio 2019 07:28:54 UTC+1, Burak Serdar ha scritto:
>
> On Tue, Feb 5, 2019 at 8:13 PM robert engels > wrote:
> >
> > That’s what I was trying to point out. Your design is not correct. The
> Body is a Reader, not a Buffer - the
On Tue, Feb 5, 2019 at 8:13 PM robert engels wrote:
>
> That’s what I was trying to point out. Your design is not correct. The Body
> is a Reader, not a Buffer - the length of the request/body may be
> indeterminate - that is, a stream. Attempting to get the length of an
> underlying buffer is
That’s what I was trying to point out. Your design is not correct. The Body is
a Reader, not a Buffer - the length of the request/body may be indeterminate -
that is, a stream. Attempting to get the length of an underlying buffer is not
only probably not possible, but not correct in many
On Tue, Feb 5, 2019 at 7:00 PM Robert Engels wrote:
>
> Shouldn’t you just be taking the content length from the header if forwarding
> the same body. There is no need for the length of the body.
True. What I was suggesting is a fix for the general case.
>
> > On Feb 5, 2019, at 6:53 PM, Burak
Shouldn’t you just be taking the content length from the header if forwarding
the same body. There is no need for the length of the body.
> On Feb 5, 2019, at 6:53 PM, Burak Serdar wrote:
>
>> On Tue, Feb 5, 2019 at 5:18 PM Dan Kortschak wrote:
>>
>> Personally, I think this is a bug in the
On Tue, Feb 5, 2019 at 5:18 PM Dan Kortschak wrote:
>
> Personally, I think this is a bug in the behaviour of NewRequest. See h
> ttps://github.com/golang/go/issues/18117 for some additional context.
Agreed. One solution could be to have:
type HasLen interface {
int Len()
}
Then have
Personally, I think this is a bug in the behaviour of NewRequest. See h
ttps://github.com/golang/go/issues/18117 for some additional context.
On Tue, 2019-02-05 at 05:18 -0800, matteo.biage...@gmail.com wrote:
> I've the following situation:
> I proxy a request to another server and when I made
I think we’ll just agree to disagree.
> On Feb 5, 2019, at 2:52 PM, Burak Serdar wrote:
>
>> On Tue, Feb 5, 2019 at 1:46 PM Robert Engels wrote:
>>
>> If the error doesn’t know what it is wrapping but the caller needs to know
>> the wrapped error you have a design problem. It breaks all
On Tue, Feb 5, 2019 at 1:46 PM Robert Engels wrote:
>
> If the error doesn’t know what it is wrapping but the caller needs to know
> the wrapped error you have a design problem. It breaks all sorts of ‘isa’
> semantics.
This happens if you're using multiple layers of libraries that don't
know
If the error doesn’t know what it is wrapping but the caller needs to know the
wrapped error you have a design problem. It breaks all sorts of ‘isa’
semantics.
If you need to know the internal buffer length, you have a design problem. What
if the wrapped buffer was unlimited or dynamic -
On Tue, Feb 5, 2019 at 12:22 PM Robert Engels wrote:
>
> As far as the error handling, this is kind of a limitation of the error
> interface in Go and should be solved there. But I would start with asking,
> why do you need the causation error? If it is increased logging, then the
> higher
As far as the error handling, this is kind of a limitation of the error
interface in Go and should be solved there. But I would start with asking, why
do you need the causation error? If it is increased logging, then the higher
level error should be able to represent the additional detail when
On Tue, Feb 5, 2019 at 11:27 AM Robert Engels wrote:
>
> But even then, exposing internal details outward leads to lots of problems,
> testability for certain. There’s almost always a better way then GetNested,
> GetWrapped, or any of its derivatives.
How so? How would you deal with something
On Tue, Feb 5, 2019 at 11:20 AM Robert Engels wrote:
>
> Then you want inheritance not encapsulation.
No, this is done at runtime, not at compile time.
>
> > On Feb 5, 2019, at 10:46 AM, Burak Serdar wrote:
> >
> >> On Tue, Feb 5, 2019 at 9:41 AM Robert Engels wrote:
> >>
> >> GetNested
But even then, exposing internal details outward leads to lots of problems,
testability for certain. There’s almost always a better way then GetNested,
GetWrapped, or any of its derivatives.
> On Feb 5, 2019, at 12:20 PM, Robert Engels wrote:
>
> Then you want inheritance not encapsulation.
Then you want inheritance not encapsulation.
> On Feb 5, 2019, at 10:46 AM, Burak Serdar wrote:
>
>> On Tue, Feb 5, 2019 at 9:41 AM Robert Engels wrote:
>>
>> GetNested anything is a sign something is broken in the API. The whole point
>> of being nested is almost always encapsulation and
On Tue, Feb 5, 2019 at 9:41 AM Robert Engels wrote:
>
> GetNested anything is a sign something is broken in the API. The whole point
> of being nested is almost always encapsulation and then you are breaking it.
That's too much of a generalization in my opinion. This is more like
decoration,
GetNested anything is a sign something is broken in the API. The whole point of
being nested is almost always encapsulation and then you are breaking it.
> On Feb 5, 2019, at 10:30 AM, Burak Serdar wrote:
>
>> On Tue, Feb 5, 2019 at 9:12 AM wrote:
>>
>> I've the following situation:
>> I
On Tue, Feb 5, 2019 at 9:12 AM wrote:
>
> I've the following situation:
> I proxy a request to another server and when I made a POST and create a new
> request, the contentLength is zero:
>
> req2, _ := http.NewRequest(req.Method, newApiUrl , req.Body)
> fmt.Println("New request
I've the following situation:
I proxy a request to another server and when I made a POST and create a new
request, the contentLength is zero:
req2, _ := http.NewRequest(req.Method, newApiUrl , req.Body)
fmt.Println("New request from body:", req2.ContentLength) // print 0
40 matches
Mail list logo
