<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/docker.git/errdefs, branch master</title>
<subtitle>github.com: dotcloud/docker.git
</subtitle>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/docker.git/'/>
<entry>
<title>errdefs: FromStatusCode() don't log "FIXME" debug message</title>
<updated>2022-12-20T15:03:46+00:00</updated>
<author>
<name>Sebastiaan van Stijn</name>
<email>github@gone.nl</email>
</author>
<published>2022-12-20T15:03:46+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/docker.git/commit/?id=2e67c827bb5362171ab574084974c080b13e2da9'/>
<id>2e67c827bb5362171ab574084974c080b13e2da9</id>
<content type='text'>
This utility is used by the client, which cannot do anything about errors
received from the API. In situations where no API connection was possible,
for example, if the client has no permissions to connect to the socket,
the request would have a "-1" status-code;
https://github.com/moby/moby/blob/3e39ec60dab859f70577ea8561fe5bda5236749e/client/request.go#L133-L134

In this case, a client with "debug" enabled, would print _and_ log a confusing
error message:

    DEBU[0000] FIXME: Got an status-code for which error does not match any expected type!!!  error="Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Post \"http://%2Fvar%2Frun%2Fdocker.sock/v1.24/build?buildargs=%7B%7D&amp;cachefrom=%5B%5D&amp;cgroupparent=&amp;cpuperiod=0&amp;cpuquota=0&amp;cpusetcpus=&amp;cpusetmems=&amp;cpushares=0&amp;dockerfile=Dockerfile.repro&amp;labels=%7B%7D&amp;memory=0&amp;memswap=0&amp;networkmode=default&amp;rm=1&amp;shmsize=0&amp;t=repro&amp;target=&amp;ulimits=null&amp;version=1\": dial unix /var/run/docker.sock: connect: permission denied" module=api status_code=-1
    Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Post "http://%2Fvar%2Frun%2Fdocker.sock/v1.24/build?buildargs=%7B%7D&amp;cachefrom=%5B%5D&amp;cgroupparent=&amp;cpuperiod=0&amp;cpuquota=0&amp;cpusetcpus=&amp;cpusetmems=&amp;cpushares=0&amp;dockerfile=Dockerfile.repro&amp;labels=%7B%7D&amp;memory=0&amp;memswap=0&amp;networkmode=default&amp;rm=1&amp;shmsize=0&amp;t=repro&amp;target=&amp;ulimits=null&amp;version=1": dial unix /var/run/docker.sock: connect: permission denied

In the above; `DEBU` logs the error (including the "FIXME"), and the second
line is the error message printed.

This was a mistake on my side when I added the `FromStatusCode` utility. I
implemented that to be the counterpart to `FromError`, but in doing so also
copied over the logging (see 1af30c50ca0ad81e2839ff4f2c5e70413f021d52). That
log-message is only intended to be logged on the daemon side, for situations
where we return an error without a proper errdefs (which would result in an
500 "internal server error" to be returned by the API).

This patch removes the debug log, and a minor cleanup to explicitly return
"nil" if we didn't get an error in the first place.

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This utility is used by the client, which cannot do anything about errors
received from the API. In situations where no API connection was possible,
for example, if the client has no permissions to connect to the socket,
the request would have a "-1" status-code;
https://github.com/moby/moby/blob/3e39ec60dab859f70577ea8561fe5bda5236749e/client/request.go#L133-L134

In this case, a client with "debug" enabled, would print _and_ log a confusing
error message:

    DEBU[0000] FIXME: Got an status-code for which error does not match any expected type!!!  error="Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Post \"http://%2Fvar%2Frun%2Fdocker.sock/v1.24/build?buildargs=%7B%7D&amp;cachefrom=%5B%5D&amp;cgroupparent=&amp;cpuperiod=0&amp;cpuquota=0&amp;cpusetcpus=&amp;cpusetmems=&amp;cpushares=0&amp;dockerfile=Dockerfile.repro&amp;labels=%7B%7D&amp;memory=0&amp;memswap=0&amp;networkmode=default&amp;rm=1&amp;shmsize=0&amp;t=repro&amp;target=&amp;ulimits=null&amp;version=1\": dial unix /var/run/docker.sock: connect: permission denied" module=api status_code=-1
    Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Post "http://%2Fvar%2Frun%2Fdocker.sock/v1.24/build?buildargs=%7B%7D&amp;cachefrom=%5B%5D&amp;cgroupparent=&amp;cpuperiod=0&amp;cpuquota=0&amp;cpusetcpus=&amp;cpusetmems=&amp;cpushares=0&amp;dockerfile=Dockerfile.repro&amp;labels=%7B%7D&amp;memory=0&amp;memswap=0&amp;networkmode=default&amp;rm=1&amp;shmsize=0&amp;t=repro&amp;target=&amp;ulimits=null&amp;version=1": dial unix /var/run/docker.sock: connect: permission denied

In the above; `DEBU` logs the error (including the "FIXME"), and the second
line is the error message printed.

This was a mistake on my side when I added the `FromStatusCode` utility. I
implemented that to be the counterpart to `FromError`, but in doing so also
copied over the logging (see 1af30c50ca0ad81e2839ff4f2c5e70413f021d52). That
log-message is only intended to be logged on the daemon side, for situations
where we return an error without a proper errdefs (which would result in an
500 "internal server error" to be returned by the API).

This patch removes the debug log, and a minor cleanup to explicitly return
"nil" if we didn't get an error in the first place.

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>errdefs: move GetHTTPErrorStatusCode to api/server/httpstatus</title>
<updated>2022-03-21T11:22:39+00:00</updated>
<author>
<name>Sebastiaan van Stijn</name>
<email>github@gone.nl</email>
</author>
<published>2022-03-21T10:27:39+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/docker.git/commit/?id=e82b7b2fa00d1f30f3eabb58eaf7d74806581369'/>
<id>e82b7b2fa00d1f30f3eabb58eaf7d74806581369</id>
<content type='text'>
This reverts the changes made in 2a9c987e5a72549775ffa4dc31595ceff4f06a78, which
moved the GetHTTPErrorStatusCode() utility to the errdefs package.

While it seemed to make sense at the time to have the errdefs package provide
conversion both from HTTP status codes errdefs and the reverse, a side-effect
of the move was that the errdefs package now had a dependency on various external
modules, to handle conversio of errors coming from those sub-systems, such as;

- github.com/containerd/containerd
- github.com/docker/distribution
- google.golang.org/grpc

This patch moves the conversion from (errdef-) errors to HTTP status-codes to a
 api/server/httpstatus package, which is only used by the API server, and should
not be needed by client-code using the errdefs package.

The MakeErrorHandler() utility was moved to the API server itself, as that's the
only place it's used. While the same applies to the GetHTTPErrorStatusCode func,
I opted for keeping that in its own package for a slightly cleaner interface.

Why not move it into the api/server/httputils package?

The api/server/httputils package is also imported in the client package, which
uses the httputils.ParseForm() and httputils.HijackConnection() functions as
part of the TestTLSCloseWriter() test. While this is only used in tests, I
wanted to avoid introducing the indirect depdencencies outside of the api/server
code.

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This reverts the changes made in 2a9c987e5a72549775ffa4dc31595ceff4f06a78, which
moved the GetHTTPErrorStatusCode() utility to the errdefs package.

While it seemed to make sense at the time to have the errdefs package provide
conversion both from HTTP status codes errdefs and the reverse, a side-effect
of the move was that the errdefs package now had a dependency on various external
modules, to handle conversio of errors coming from those sub-systems, such as;

- github.com/containerd/containerd
- github.com/docker/distribution
- google.golang.org/grpc

This patch moves the conversion from (errdef-) errors to HTTP status-codes to a
 api/server/httpstatus package, which is only used by the API server, and should
not be needed by client-code using the errdefs package.

The MakeErrorHandler() utility was moved to the API server itself, as that's the
only place it's used. While the same applies to the GetHTTPErrorStatusCode func,
I opted for keeping that in its own package for a slightly cleaner interface.

Why not move it into the api/server/httputils package?

The api/server/httputils package is also imported in the client package, which
uses the httputils.ParseForm() and httputils.HijackConnection() functions as
part of the TestTLSCloseWriter() test. While this is only used in tests, I
wanted to avoid introducing the indirect depdencencies outside of the api/server
code.

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>log error message when receiving an unexpected type error</title>
<updated>2021-11-09T09:53:08+00:00</updated>
<author>
<name>tonic</name>
<email>tonicbupt@gmail.com</email>
</author>
<published>2021-11-07T15:12:30+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/docker.git/commit/?id=24aaa7f8c9c948f58e1326a044ae8978b0b19f1f'/>
<id>24aaa7f8c9c948f58e1326a044ae8978b0b19f1f</id>
<content type='text'>
Signed-off-by: tonic &lt;tonicbupt@gmail.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Signed-off-by: tonic &lt;tonicbupt@gmail.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Implement Unwrap to errors</title>
<updated>2020-11-21T15:36:35+00:00</updated>
<author>
<name>Kostadin Plachkov</name>
<email>k.n.plachkov@gmail.com</email>
</author>
<published>2020-11-15T19:41:37+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/docker.git/commit/?id=aeddf93de068bc02926c07633840cb5ac4a41329'/>
<id>aeddf93de068bc02926c07633840cb5ac4a41329</id>
<content type='text'>
Signed-off-by: Kostadin Plachkov &lt;k.n.plachkov@gmail.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Signed-off-by: Kostadin Plachkov &lt;k.n.plachkov@gmail.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>bump gotest.tools v3.0.1 for compatibility with Go 1.14</title>
<updated>2020-02-10T23:06:42+00:00</updated>
<author>
<name>Sebastiaan van Stijn</name>
<email>github@gone.nl</email>
</author>
<published>2020-02-07T13:39:24+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/docker.git/commit/?id=9f0b3f5609faf69537cdf1882c251deec6e9eb26'/>
<id>9f0b3f5609faf69537cdf1882c251deec6e9eb26</id>
<content type='text'>
full diff: https://github.com/gotestyourself/gotest.tools/compare/v2.3.0...v3.0.1

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
full diff: https://github.com/gotestyourself/gotest.tools/compare/v2.3.0...v3.0.1

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge pull request #39527 from thaJeztah/pull_platform_regression</title>
<updated>2019-07-16T01:29:16+00:00</updated>
<author>
<name>Sebastiaan van Stijn</name>
<email>thaJeztah@users.noreply.github.com</email>
</author>
<published>2019-07-16T01:29:16+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/docker.git/commit/?id=81dbed4c8b87d99824eecea68c95f10056fd3d2a'/>
<id>81dbed4c8b87d99824eecea68c95f10056fd3d2a</id>
<content type='text'>
Fix error handling of incorrect --platform values</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Fix error handling of incorrect --platform values</pre>
</div>
</content>
</entry>
<entry>
<title>Add regression tests for invalid platform status codes</title>
<updated>2019-07-15T18:37:00+00:00</updated>
<author>
<name>Sebastiaan van Stijn</name>
<email>github@gone.nl</email>
</author>
<published>2019-07-15T16:19:31+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/docker.git/commit/?id=9d1b4f5fc3fd647ac4f258374ee5369ffab85c0d'/>
<id>9d1b4f5fc3fd647ac4f258374ee5369ffab85c0d</id>
<content type='text'>
Before we handled containerd errors, using an invalid platform produced a 500 status:

```bash
curl -v \
  -X POST \
  --unix-socket /var/run/docker.sock \
  "http://localhost:2375/v1.40/images/create?fromImage=hello-world&amp;platform=foobar&amp;tag=latest" \
  -H "Content-Type: application/json"
```

```
* Connected to localhost (docker.sock) port 80 (#0)
&gt; POST /v1.40/images/create?fromImage=hello-world&amp;platform=foobar&amp;tag=latest HTTP/1.1
&gt; Host: localhost:2375
&gt; User-Agent: curl/7.54.0
&gt; Accept: */*
&gt; Content-Type: application/json
&gt;
&lt; HTTP/1.1 500 Internal Server Error
&lt; Api-Version: 1.40
&lt; Content-Length: 85
&lt; Content-Type: application/json
&lt; Date: Mon, 15 Jul 2019 15:25:44 GMT
&lt; Docker-Experimental: true
&lt; Ostype: linux
&lt; Server: Docker/19.03.0-rc2 (linux)
&lt;
{"message":"\"foobar\": unknown operating system or architecture: invalid argument"}
```

That problem is now fixed, and the API correctly returns a 4xx status:

```bash
curl -v \
  -X POST \
  --unix-socket /var/run/docker.sock \
  "http://localhost:2375/v1.40/images/create?fromImage=hello-world&amp;platform=foobar&amp;tag=latest" \
  -H "Content-Type: application/json"
```

```
* Connected to localhost (/var/run/docker.sock) port 80 (#0)
&gt; POST /v1.40/images/create?fromImage=hello-world&amp;platform=foobar&amp;tag=latest HTTP/1.1
&gt; Host: localhost:2375
&gt; User-Agent: curl/7.52.1
&gt; Accept: */*
&gt; Content-Type: application/json
&gt;
&lt; HTTP/1.1 400 Bad Request
&lt; Api-Version: 1.41
&lt; Content-Type: application/json
&lt; Docker-Experimental: true
&lt; Ostype: linux
&lt; Server: Docker/dev (linux)
&lt; Date: Mon, 15 Jul 2019 15:13:42 GMT
&lt; Content-Length: 85
&lt;
{"message":"\"foobar\": unknown operating system or architecture: invalid argument"}
* Curl_http_done: called premature == 0
```

This patch adds tests to validate the behaviour

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Before we handled containerd errors, using an invalid platform produced a 500 status:

```bash
curl -v \
  -X POST \
  --unix-socket /var/run/docker.sock \
  "http://localhost:2375/v1.40/images/create?fromImage=hello-world&amp;platform=foobar&amp;tag=latest" \
  -H "Content-Type: application/json"
```

```
* Connected to localhost (docker.sock) port 80 (#0)
&gt; POST /v1.40/images/create?fromImage=hello-world&amp;platform=foobar&amp;tag=latest HTTP/1.1
&gt; Host: localhost:2375
&gt; User-Agent: curl/7.54.0
&gt; Accept: */*
&gt; Content-Type: application/json
&gt;
&lt; HTTP/1.1 500 Internal Server Error
&lt; Api-Version: 1.40
&lt; Content-Length: 85
&lt; Content-Type: application/json
&lt; Date: Mon, 15 Jul 2019 15:25:44 GMT
&lt; Docker-Experimental: true
&lt; Ostype: linux
&lt; Server: Docker/19.03.0-rc2 (linux)
&lt;
{"message":"\"foobar\": unknown operating system or architecture: invalid argument"}
```

That problem is now fixed, and the API correctly returns a 4xx status:

```bash
curl -v \
  -X POST \
  --unix-socket /var/run/docker.sock \
  "http://localhost:2375/v1.40/images/create?fromImage=hello-world&amp;platform=foobar&amp;tag=latest" \
  -H "Content-Type: application/json"
```

```
* Connected to localhost (/var/run/docker.sock) port 80 (#0)
&gt; POST /v1.40/images/create?fromImage=hello-world&amp;platform=foobar&amp;tag=latest HTTP/1.1
&gt; Host: localhost:2375
&gt; User-Agent: curl/7.52.1
&gt; Accept: */*
&gt; Content-Type: application/json
&gt;
&lt; HTTP/1.1 400 Bad Request
&lt; Api-Version: 1.41
&lt; Content-Type: application/json
&lt; Docker-Experimental: true
&lt; Ostype: linux
&lt; Server: Docker/dev (linux)
&lt; Date: Mon, 15 Jul 2019 15:13:42 GMT
&lt; Content-Length: 85
&lt;
{"message":"\"foobar\": unknown operating system or architecture: invalid argument"}
* Curl_http_done: called premature == 0
```

This patch adds tests to validate the behaviour

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>errdefs: convert containerd errors to the correct status code</title>
<updated>2019-07-15T18:36:57+00:00</updated>
<author>
<name>Sebastiaan van Stijn</name>
<email>github@gone.nl</email>
</author>
<published>2019-07-15T16:15:45+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/docker.git/commit/?id=4a516215e26542b723fe2d74c6c196a366164473'/>
<id>4a516215e26542b723fe2d74c6c196a366164473</id>
<content type='text'>
In situations where the containerd error is consumed directly
and not received over gRPC, errors were not translated.

This patch converts containerd errors to the correct HTTP
status code.

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
In situations where the containerd error is consumed directly
and not received over gRPC, errors were not translated.

This patch converts containerd errors to the correct HTTP
status code.

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>errdefs: remove unneeded recursive calls</title>
<updated>2019-07-15T16:22:19+00:00</updated>
<author>
<name>Sebastiaan van Stijn</name>
<email>github@gone.nl</email>
</author>
<published>2019-07-15T15:09:20+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/docker.git/commit/?id=32f4fdfb5c8846922302c20fdcab973227eb9bd6'/>
<id>32f4fdfb5c8846922302c20fdcab973227eb9bd6</id>
<content type='text'>
The `statusCodeFromGRPCError` and `statusCodeFromDistributionError`
helpers are used by `GetHTTPErrorStatusCode`, which already recurses
if the error implements the `Causer` interface.

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The `statusCodeFromGRPCError` and `statusCodeFromDistributionError`
helpers are used by `GetHTTPErrorStatusCode`, which already recurses
if the error implements the `Causer` interface.

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>errdefs: remove "ErrAlreadyExists" because it's not an error</title>
<updated>2019-03-21T21:25:15+00:00</updated>
<author>
<name>Sebastiaan van Stijn</name>
<email>github@gone.nl</email>
</author>
<published>2018-12-31T12:10:29+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/docker.git/commit/?id=7d4b7883815d45a0b4e1f4c50ec566e24040cb2a'/>
<id>7d4b7883815d45a0b4e1f4c50ec566e24040cb2a</id>
<content type='text'>
The `ErrAlreadyExists` error is used for 304 statuses, which
is not an error-condition, so should probably not be defined
as part of the errdefs package.

This patch removes the `ErrAlreadyExists` interface, and related
helpers, as it was currently not used.

Note that a 304 status can fulfil certain use-cases, but (refering
to https://www.codetinkerer.com/2015/12/04/choosing-an-http-status-code.html)
could probably be handled by a 200 OK, unless we want to perform
caching in the client.

If we do want to use 304 statuses, perhaps we need a separate class
of "errors" for this (?).

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The `ErrAlreadyExists` error is used for 304 statuses, which
is not an error-condition, so should probably not be defined
as part of the errdefs package.

This patch removes the `ErrAlreadyExists` interface, and related
helpers, as it was currently not used.

Note that a 304 status can fulfil certain use-cases, but (refering
to https://www.codetinkerer.com/2015/12/04/choosing-an-http-status-code.html)
could probably be handled by a 200 OK, unless we want to perform
caching in the client.

If we do want to use 304 statuses, perhaps we need a separate class
of "errors" for this (?).

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
</pre>
</div>
</content>
</entry>
</feed>
