Subject: debug field in GOAWAY

From: Tatsuhiro Tsujikawa <[email-masked]>
Date: Fri, 21 Feb 2014 23:36:10 +0900

We have 17 implementations so far. Are there anyone who use debug field in
GOAWAY in a meaningful way?
If the answer is no, is there any possibility to remove it from the spec?
So that we have 1 less complexity.

Best regards,
Tatsuhiro Tsujikawa

---

From: Cory Benfield <[email-masked]>
Date: Fri, 21 Feb 2014 14:45:13 +0000

> GOAWAY in a meaningful way?

Hyper doesn't use it, and the most it'd ever do with it is throw it in
exceptions or print it in logs: I doubt it will ever take action on
it.

However, on a purely selfish level I don't object to having it there.
It doesn't add any significant complexity to my code or to my users'
code, and there's no spec requirement that I pay any attention to it
at all. From a pure implementation standpoint I treat that field as
opaque data, and so I'm happy to take it or leave it.

Of course, if someone has a philosophical reason to remove it or keep
it in I'll happily defer to their wisdom.

Cory

---

From: Hasan Khalil <[email-masked]>
Date: Fri, 21 Feb 2014 15:47:20 +0000

Google have implemented it, but not launched anything to the general public
yet. It has been useful during development, though, and I expect it to be
very useful in deployment as well.

The complexity here is, as stated, very low.

---

From: Tatsuhiro Tsujikawa <[email-masked]>
Date: Sat, 22 Feb 2014 00:48:00 +0900

Hi,

> it in I'll happily defer to their wisdom.

My argument is that if debug field is just there and not cared by the
receiver and is just discarded, then there is no point to keep it in the
protocol specification.

If you cannot expect anything in that field from the other implementations
because it is opaque and completely undefined.
On the other hand, if a person or group is developing a pair of client and
server, then their own logging is much more useful in my opinion.

Best regards,
Tatsuhiro Tsujikawa

---

From: Ilari Liusvaara <[email-masked]>
Date: Fri, 21 Feb 2014 17:52:35 +0200

> it in I'll happily defer to their wisdom.

Well, there are a lot of uses of GOAWAY(PROTOCOL_ERROR). It could be
helpful to know which was violated when debugging an implementation.

Seraching PROTOCOL_ERROR from spec turns up that receiving anything
of following will cause GOAWAY(PRTOCOL_ERROR):

- Frame of unknown type.
- Anything except PRIORITY or RST_STREAM in reserved (local).
- Anything except HEADERS or RST_STReAM in reserved (remote).
- Anything(?) signficantly after stream close (MAY).
- Frame in unexpected state.
- Frame opening stream with stream not greater than all previous.
- Frame with PAD_HIGH,!PAD_LOW.
- DATA frame on stream 0.
- Frame with more padding than size.
- Anything except CONTINUATION with active header block.
- Anything on another stream with active header block.
- HEADERS on stream 0.
- PRIORITY on stream 0.
- RST_STREAM on stream 0.
- RST_STREAM on idle stream.
- SETTINGS on stream other than 0.
- "Badly formed or incomplete" SETTINGS.
- PUSH_PROMISE with pushing disabled.
- SETTINGS_ENABLE_PUSH anything except 0 or 1.
- SETTINGS with unknown setting.
- PUSH_PROMISE associated with stream 0.
- PUSH_PROMISE associated with stream not open down.
- PUSH_PROMISE promising non-idle stream.
- PING on stream other than 0.
- GOAWAY on stream other than 0.
- CONTINUATION without active header block.
- PUSH_PROMISE on server.
- SETTINGS from server that enables pushing.

Then there's RST_STREAM(PROTOCOL_ERROR), but it is only used in
one place, namely receiving incomplete or with invalid method
PUSH_PROMISE.

-Ilari

---

From: Cory Benfield <[email-masked]>
Date: Fri, 21 Feb 2014 17:32:44 +0000

> helpful to know which was violated when debugging an implementation.

Agreed, but I don't believe that a free-text field is the way to do
that. If we really care about making it easy to debug PROTOCOL_ERRORs
we should be adding subcodes or increasing the number of error codes.
I would be much more enthusiastically in favour of error subcodes than
anything else.

Now, if we want to develop a convention of assigning subcodes to
specific errors and putting them in the debug field, that's an
acceptable thing to do, but if we think that convention is useful why
not codify it?

Anyway, as I said before, I have no objection to keeping the debug
info around, it isn't causing me any problems at all.

Cory

---

From: Martin Thomson <[email-masked]>
Date: Fri, 21 Feb 2014 09:43:56 -0800

> acceptable thing to do, but if we think that convention is useful why

Agree.  In principle.

However, from hard experience on this, getting this right is a lot of
work.  Even if one person is willing to sit down and build a taxonomy
and derive from that a more extensive list of error codes, it's hard.
Primarily, it's hard to get agreement that the taxonomy is right in
both structure and level of detail.  People have different
implementations, with different structures and what makes sense in one
implementation might be completely wrong in another.

Over time, I've concluded that - for standardization - providing the
minimal set of error codes is best.  And by minimal I mean that every
code implies a different action.  For PROTOCOL_ERROR, the worst of our
codes, this action is "fix your broken implementation".  If you can
identify a case that might have a different action, please raise that
as an issue.

---

From: Adrian Cole <[email-masked]>
Date: Sat, 22 Feb 2014 09:03:45 -0800

Hehe:

      if (debugData.size() > 0) { // TODO: log the debugData
      }

That said, I wouldn't read too much into it.  The data is usually an
exception message.

-A

On Fri, Feb 21, 2014 at 6:36 AM, Tatsuhiro Tsujikawa
<[email-masked]> wrote:
> If the answer is no, is there any possibility to remove it from the spec? So
> that we have 1 less complexity.
>
> Best regards,
> Tatsuhiro Tsujikawa
>

---

From: Cory Benfield <[email-masked]>
Date: Sat, 22 Feb 2014 17:58:24 +0000

> That said, I wouldn't read too much into it. The data is usually an

In a similar vein, after everything I said yesterday I made a commit
to hyper today[1] that prints the debugging data field when we get a
GOAWAY to help me debug an interop problem I was having with Twitter.
It didn’t help, but it reinforces the idea that it might be useful if
more implementations take the time to fill it in.

[1]: https://github.com/Lukasa/hyper/commit/77f0cd88f623cc5015e10930cd26c31f5b574db1