adding slog for additional logging about appsync-client-go

[KazuhitoNakamura]

Hello, @jeremyforan.

Thank you for your suggestions.
I support the introduction of slog. I have experience using slog in another project, so I understand its benefits.

[jeremyforan]

Hello @KazuhitoNakamura,

I will make a pull request. Also, I have made a small change for you to consider.

func (r *realtimeWebSocketOperation) readLoop() {
...
_, payload, err := r.ws.ReadMessage()
if err != nil {
	if netErr, ok := err.(net.Error); ok && netErr.Timeout() {
		slog.Warn("connection timeout")
	} else {
		slog.ErrorContext(r.ctx, "error reading message", "error", err)
	}
	r.onConnectionLost(err)
	return
}

We were getting intermittent "connection reset by peer" message. It looks like AWS is closing the session without warning. This would happen after a day or two of a single websocket being open. Because the error was not a timeout the onConnectionLost wasn't be called.

The change will always result in r.onConnectionLost(err) on a r.ws.ReadMessage() error. What do you think? Is there other considerations to be made here?

[KazuhitoNakamura]

Thank you so much for your pull request, I have confirmed the pull request.

I don't think this change will result in the expected behavior since onConnectionLost is always called when a ReadMessage error occurs and is not limited to the error we want to resolve this time.

Is it possible to know the type of error that occurred this time?

[jeremyforan]

It is difficult to capture the error as it is intermittent. It can take 2-3 days to occur. Currently we have a ticket open with AWS and they have escalated this issue to the AppSync team.

I tried using the syscall.ECONNRESET error type, based on this article https://gosamples.dev/connection-reset-by-peer/ but it did not seem to capture the error in testing:

if errors.Is(err, syscall.ECONNRESET) {
            log.Print("This is connection reset by peer error")
        }

Is there a efficient way to validate the connection is still valid? That way if there is an error we can check if the connection is still valid.

_, payload, err := r.ws.ReadMessage()
if err != nil {
	if netErr, ok := err.(net.Error); ok && netErr.Timeout() {
		slog.Warn("connection timeout")
		r.onConnectionLost(err)
		return
	} else {
		// check if connection has been lost
		slog.ErrorContext(r.ctx, "error reading message", "error", err)
		r.onConnectionLost(err)
		return
	}
		slog.Warn("error reading message", "error", err)
	return
}

[KazuhitoNakamura]

I apologize for the late reply.
I’m afraid, I'm not sure that way to check if connection has been lost for now.
I will read the source code to see if there are any errors that validate the connection has been broken.

To introduce slog ahead of time, could you remove this small change from the #31?

[jeremyforan]

Hey @KazuhitoNakamura

I agree with you.

I will push the changes as per your comment so that we can include the slog functionality. I will push the changes shortly.

[KazuhitoNakamura]

I found it quite difficult to check if the connection was lost because there are several errors defined in errors.New().

• ReadMessage()
  • NextReader()
    • advanceFrame()
      • CopyN()
        • Copy()
          • copyBuffer()
            • Read()
              • Read()
                • Read()
                  • syscall.EINVAL
                  • OpError or io.EOF
            • errInvalidWrite
            • ErrShortWrite
        • EOF
      • read()
        • Peek()
          • Peek()
            • ErrNegativeCount
            • ErrBufferFull
            • io.ErrNoProgress
        • errUnexpectedEOF
      • handleProtocolError()
        • handleProtocolError()
      • setReadRemaining()
        • setReadRemaining()
          • ErrReadLimit
      • handlePing
        • WriteControl
          • errBadWriteOpCode
          • errInvalidControlFrame
          • errWriteTimeout

...

[jeremyforan]

merge complete.