fix content-type declared by /events API by ndeloof · Pull Request #50953 · moby/moby

ndeloof · 2025-09-11T10:57:20Z

- What I did

fixed /events API to report content-type as application/x-ndjson
using application/json in this API is wrong, I assume client always ignore Content-Type set here otherwise they would only unmarshal the first object in the stream and miss all others.

There's unfortunately no official mime type for line-delimited json object streams

- How I did it

- How to verify it

- Human readable description for the release notes

`events` API now reports content-type as `application/x-ndjson` for newline-delimited JSON event stream

- A picture of a cute animal (not mandatory but encouraged)

akerouanton · 2025-09-11T11:34:45Z

Now that the api is a separate module, we need to re-vendor changes made in the api folder to make them available to the daemon module. That's why the CI is currently failing to build your PR. (You just need to run go mod vendor.)

You also need to update the content type declared in the Swagger file:

moby/api/swagger.yaml

Lines 10351 to 10352 in 3278393

    
           produces: 
        
             - "application/json"

And add a note in the API changelog: https://github.com/moby/moby/blob/master/api/docs/CHANGELOG.md

ndeloof · 2025-09-11T11:49:26Z

You also need to update the content type declared in the Swagger file:

stupid me, this is the reason I've been looking into this, and eventually forgot to fix 😅

thaJeztah · 2025-09-11T12:59:55Z

This probably also requires changes in the swagger definition to declare the content-types it can produce.

I recall you had another PR related to delimited JSON responses, but that one used application/json-seq - I think application/json-seq was an RFC / formal standard, and application/x-ndjson not an official one (also using the x- prefix which is now discouraged / deprecated)

postImagesCreate does actually produce JSON sequence (rfc7464). #43157

ndeloof · 2025-09-13T10:29:29Z

@thaJeztah application/json-seq isn't actually a newline delimited JSON stream.
Json-seq uses a record separator (ASCII 0x1E) to allow multi-line json objects, while nd-json just assumes json object matches a single line. Could be interesting API uses Accept header to select which one to use, but this is out of the scope for this PR (see With golang/go#19307)

Media type used here is the one recommended by https://github.com/ndjson/ndjson-spec?tab=readme-ov-file#33-mediatype-and-file-extensions

ndeloof · 2025-09-17T06:56:36Z

I just noticed we already rely on unofficial x- mime types for TAR

thaJeztah · 2025-09-19T12:21:45Z

I just noticed we already rely on unofficial x- mime types for TAR

Yes, we debated at the time; I think before that we used application/octet-stream, because there is no official mediaType for tar (someone should probably register one in the IANA); application/x-tar is closest, and considered a "common" mediaType; https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/MIME_types/Common_types

(I think using x- prefixes as "intermediate, not-yet-official" mediatype is discouraged nowadays, as moving from non-official to official would cause things to diverge; and adding a x- prefix without going through the official bodies to register would still have the chance to conflict with other uses).

Could be interesting API uses Accept header to select which one to use, but this is out of the scope for this PR (see With golang/go#19307)

Oh! I missed that this is (finally!) getting some traction; I think technically we don't have to wait for that proposal to land; we already have some code in place (using the very old gddo dependency, but it's doing it's job fine);

moby/daemon/server/router/container/copy.go

Lines 46 to 48 in 5a69e91

    
           func writeCompressedResponse(w http.ResponseWriter, r *http.Request, body io.Reader) error { 
        
           	var cw io.Writer 
        
           	switch gddohttputil.NegotiateContentEncoding(r, []string{"gzip", "deflate"}) {

Digging a bit deeper in the options we have; during the maintainers call, @corhere also contributed another option (use SSE as a response), which I'll add below.

`application/json-seq` (RFC7464)

RFC7464 (application/json-seq) is a more robust, because the record-separator cannot appear in JSON documents; this also (technically) would allow us to return pretty-printed JSON (not something we likely want though), or to split large JSON structs (which COULD be interesting for some cases, such as the log streams). With RFC7464, newlines (LF) are optional (but recommended), so it would look like;

<RS><JSON document><LF>
<RS><JSON document><LF>

With the option for long records to;

<RS>JSON document (partial)
JSON document (partial)
JSON document (partial)
JSON document (partial)<LF>
<RS>JSON document (partial)
JSON document (partial)
JSON document (partial)
JSON document (partial)<LF>

So, ultimately it looks to be more solid, but ⚠️ it's not compatible with application/x-ndjson - clients trying to handle the response would still be able to use the LF-delimited response, but would (likely) fail to handle the record-separator (RS) and consider it part of the JSON, so malformed JSON.

Use Server-Sent Events (SSE) (`text/event-stream`)

This was opted as an alternative by @corhere; he coined using Server-Sent Events (SSE), and possibly using text/event-stream+json as mediaType; I did some searching, but as far as I could find, the +json would be non-standard, and likely break interoperability (a ;charset=utf-8 is allowed though). So if we would decide (in future) to go this path, we would probably need to stick with a plain text/event-stream, and address the "but the data is JSON" either through documentation, or with an additional (custom) header to indicate the format of the data. IFF we implement this in future, we probably should add such a header, so that we can switch between data-formats when needed.

SSE may be interesting for som of our uses; they also allow splitting messages over multiple data fields (potentially useful for large (JSON) responses), and the event type could be a match to some things we already produce.

Use `application/x-ndjson`

That's what's implemented in this PR; it's indeed for the current state of things the most appropriate mediatype. I'm concerned about unconditionally using this mediaType though;

We can gate it by API version API < 1.52 produce application/json and API > 1.52 produce application/x-ndjson
Which technically should be the client's responsibility to know they can expect a different mediatype if they decide to support API 1.52
But, it's possible that clients have code to expect application/json (and may produce errors if not), and not all clients may be explicitly selecting an API version. That's for sure a mistake on the client, but could still cause outfall, and may still be considered a breaking change at the API level.
Switching the mediaType just based on API version may also be limiting ourselves if we would want to switch between options mentioned.
👉 I think we should consider adding content-type negotiation;
- send either application/json or application/x-ndjson header based on the Accept header, and a 406 Not Acceptable if an unsupported content-type is requested.
- clients that don't send the header would continue to receive the wrong mediatype, but they were already handling this, so nothing should break
- clients that do send a Accept header can get application/x-ndjson as response.
- if we extend the options in future to add either application/json-seq or text/event-stream, we can add those options without breaking backward compatibility, and can even decide to add them regardless of API version (as the content-negotiation would be leading for clients to detect whether the option is supported if we return a 406)

thaJeztah · 2025-09-19T12:21:59Z

I want to have a look at (until the Go proposal lands) adding basic content negotiation similar to

moby/daemon/server/router/container/copy.go

Lines 46 to 48 in 5a69e91

    
           func writeCompressedResponse(w http.ResponseWriter, r *http.Request, body io.Reader) error { 
        
           	var cw io.Writer 
        
           	switch gddohttputil.NegotiateContentEncoding(r, []string{"gzip", "deflate"}) {

, as that would (per the above) keep further options open; from a quick look it should probably (famous last words) not be too complicated, and

ndeloof · 2025-09-19T12:38:53Z

content negotiation is the best option AFAICT, as this would prevent need to guard this by API version and risk for a non-compliant client to see a sudden failure to parse output.
I didn't noticed gddo was already a dependency, can use it here so a client with Accept: application/x-ndjson set will get the adequate Content-Type header set in response, while an existing client will just get the same old (but incorrect :P) application/json. I also can look into support for json-seq as a follow-up PR

thaJeztah · 2025-09-19T13:36:49Z

Oh, yes, if you have time to play; that sounds great! ❤️ - if not, it felt like a fun thing to play with in the weekend

Using gddo should be fine as long as the code is in the moby/v2 module (not in the client/ or api/ packages), as we don't really want it as a dependency for others, but for the daemon, that should all be fine.

can use it here so a client with Accept: application/x-ndjson se

As application/x-ndjson is clearly the correct content type, we can probably make the client already set it as an accept header for functions that use this / these endpoint(s). It can set an accept header for both application/x-ndjson and application/json to be "correct" (although older daemons wouldn't care).

I also can look into support for json-seq as a follow-up PR

Yup, I think that one is a "nice to have", and may require slightly more changes, but it sounds like a good option to have; depending on what language/framework is used, perhaps one is more common than the other (and vice-versa), and given that the differences are "small", we can probably support both with minimal changes.

thaJeztah

Just some quick comments from glancing over

api/types/types.go

client/internal/json-stream_test.go

daemon/server/router/json-seq.go

client/internal/json-stream.go

thaJeztah · 2025-09-23T13:13:59Z

api/types/types.go

 	MediaTypeMultiplexedStream = "application/vnd.docker.multiplexed-stream"
+
+	// MediaTypeJson is the MIME-Type for JSON objects
+	MediaTypeJson = "application/json"


nit (and to prevent linters from complaining);

Suggested change

MediaTypeJson = "application/json"

MediaTypeJSON = "application/json"

client/internal/json-stream.go

daemon/server/router/json-seq.go

client/internal/json-stream.go

thaJeztah · 2025-09-23T15:39:17Z

build was failing because 2 const references weren't updated in your last push; I did a quick rebase and fix for those to have CI run.

api/docs/CHANGELOG.md

corhere · 2025-09-23T16:55:06Z

api/swagger.yaml

      produces:
-        - "application/json"
+        - "application/x-ndjson"
+        - "application/json-seq"


I think we should leverage RFC 8091 to leave room for negotiating the format of the event content by defining a dedicated media type for the event document with the structured syntax suffix for json-seq.

Suggested change

- "application/json-seq"

- "application/vnd.docker.system-events.v1+json-seq"

👎 if content-type is expected to describe the JSON message being transported by HTTP, then each and every API endpoint would have to declare such a custom mime type (see https://datatracker.ietf.org/doc/html/rfc6839#section-3.1)

IMHO, even I don't expect most to require application/json Content-Type, this would introduce a breaking change in the API for consumers. This also makes content-type management harder to read/parse.

other (standard) options we have:

RFC 6906 profile Content-Type: application/json; profile=<URI for payload schema>

RFC 8288 Link Header Link: <URI for payload schema>; rel="describedby"

Fair enough. We already have the Engine API versioning scheme to describe the semantics of the documents.

Oh! Thought I commented, but yeah, I think adding additional mediaTypes to provide more granularity .. could be interesting, but is a bit out of scope for these initial steps. AFAICS, this PR doesn't prevent us from doing so in future if we want to, but the initial goal of this PR was to make sure the mediaTypes we return are aligning better with the implementation, and to use mediaTypes that are accepted by common framework / languages (not the default Go implementation we have).

Marking this thread as "unresolved" but only for discoverability / finding it back if we want to have that discussion in future.

thaJeztah

LGTM

thaJeztah · 2025-09-29T14:33:23Z

vendor/github.com/moby/moby/client/internal/json-stream.go

@@ -0,0 +1,97 @@
+package internal


This could probably be in some sub package, but I don't have a good suggestion for a name, and it's all internal so .. fine for later if we come up with a good name

thaJeztah · 2025-09-29T14:34:19Z

vendor/github.com/moby/moby/client/system_events.go

+		headers.Add("Accept", types.MediaTypeJSONSequence)
+		headers.Add("Accept", types.MediaTypeNDJSON)


For others; double checked that Add appends to existing values (which it does). These functions always confuse me 😂

thaJeztah · 2025-09-29T14:38:09Z

daemon/server/router/system/system_routes.go

+	contentType := httputil.NegotiateContentType(r, []string{
+		types.MediaTypeNDJSON,
+		types.MediaTypeJSONSequence,
+	}, types.MediaTypeJSON) // output isn't actually JSON but API used to  this content-type
+	w.Header().Set("Content-Type", contentType)


For others; the third argument is the "fallback". Also checked that the order in which options are taken as priority if "no q / weight" is specified by the client, so ndjson is the preferred option, and json-sequence is used if the client "accepts" it.

We could consider making ndjson the default for new API versions and only fallback to application/json for old API versions, but we can still make that change in a follow-up.

thaJeztah · 2025-09-29T14:38:51Z

api/swagger.yaml

      produces:
-        - "application/json"
+        - "application/x-ndjson"
+        - "application/json-seq"


Oh! Thought I commented, but yeah, I think adding additional mediaTypes to provide more granularity .. could be interesting, but is a bit out of scope for these initial steps. AFAICS, this PR doesn't prevent us from doing so in future if we want to, but the initial goal of this PR was to make sure the mediaTypes we return are aligning better with the implementation, and to use mediaTypes that are accepted by common framework / languages (not the default Go implementation we have).

Marking this thread as "unresolved" but only for discoverability / finding it back if we want to have that discussion in future.

corhere · 2025-10-02T19:25:47Z

client/internal/json-stream.go

+	reader io.Reader
+	buffer []byte


Would bufio.Reader simplify the implementation?

Is buffering even necessary? An io.Reader is not required to fill the destination slice. This passes the unit test:

func (r *RSFilterReader) Read(p []byte) (n int, err error) { if len(p) == 0 { return 0, nil } n, err = r.reader.Read(p) filtered := slices.DeleteFunc(p[:n], func(b byte) bool { return b == rs }) return len(filtered), err }

indeed, not sure why I came to this unnecessary complexity

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

thaJeztah · 2025-10-03T18:28:27Z

did a rebase and re-vendor to make CI happy

thaJeztah · 2025-10-03T21:11:41Z

Flaky test (should be unrelated)

=== Failed
=== FAIL: amd64.docker.docker.integration.network.bridge TestLegacyLink/no_link (0.18s)
    bridge_linux_test.go:713: assertion failed: string "Connecting to 172.18.0.2 (172.18.0.2:80)\nwget: server returned error: HTTP/1.0 404 Not Found\n" does not contain "download timed out"
    --- FAIL: TestLegacyLink/no_link (0.18s)

=== FAIL: amd64.docker.docker.integration.network.bridge TestLegacyLink (2.96s)

thaJeztah

all green now 🎉

LGTM, thanks for updating!

ndeloof force-pushed the ndjson branch from d83e0f4 to 8728081 Compare September 11, 2025 10:58

akerouanton added area/api API status/2-code-review impact/api impact/changelog kind/bugfix PR's that fix bugs labels Sep 11, 2025

ndeloof force-pushed the ndjson branch from 8728081 to 308155c Compare September 11, 2025 11:50

thaJeztah added this to 🔦 Maintainer spotlight Sep 11, 2025

github-project-automation bot moved this to New in 🔦 Maintainer spotlight Sep 11, 2025

ndeloof force-pushed the ndjson branch from 308155c to b49b9e8 Compare September 17, 2025 07:04

thaJeztah added this to the 29.0.0 milestone Sep 18, 2025

thompson-shaun moved this from New to Open in 🔦 Maintainer spotlight Sep 18, 2025

ndeloof force-pushed the ndjson branch from b49b9e8 to f229093 Compare September 22, 2025 18:22

github-actions bot added module/api module/client labels Sep 22, 2025

ndeloof force-pushed the ndjson branch 3 times, most recently from 30dc633 to 79bf1aa Compare September 23, 2025 05:24

thaJeztah reviewed Sep 23, 2025

View reviewed changes

ndeloof force-pushed the ndjson branch from 79bf1aa to c366ee7 Compare September 23, 2025 14:07

ndeloof requested a review from thaJeztah September 23, 2025 14:07

ndeloof force-pushed the ndjson branch from c366ee7 to 08a21ec Compare September 23, 2025 14:49

thaJeztah force-pushed the ndjson branch from 08a21ec to f9663e2 Compare September 23, 2025 15:38

thaJeztah force-pushed the ndjson branch from f9663e2 to cc9e0bc Compare September 23, 2025 15:44

corhere reviewed Sep 23, 2025

View reviewed changes

ndeloof force-pushed the ndjson branch from cc9e0bc to 19f06cc Compare September 23, 2025 17:14

thompson-shaun moved this from Discussing to Parked in 🔦 Maintainer spotlight Sep 25, 2025

thompson-shaun assigned ndeloof Sep 25, 2025

thaJeztah approved these changes Sep 29, 2025

View reviewed changes

corhere approved these changes Oct 2, 2025

View reviewed changes

ndeloof force-pushed the ndjson branch 2 times, most recently from a58f243 to 8dc7215 Compare October 3, 2025 06:48

negociate content-type used by /events API

df506c1

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

thaJeztah force-pushed the ndjson branch from 8dc7215 to df506c1 Compare October 3, 2025 18:28

thaJeztah approved these changes Oct 3, 2025

View reviewed changes

thaJeztah merged commit 73bacc6 into moby:master Oct 3, 2025
323 of 326 checks passed

thaJeztah mentioned this pull request Oct 3, 2025

api: docs: sync v1.52 yaml #51095

Merged

thaJeztah mentioned this pull request Feb 23, 2026

api: improve, fix content-negotiation for /events endpoint #52083

Merged

	MediaTypeJson = "application/json"
	MediaTypeJSON = "application/json"

	- "application/json-seq"
	- "application/vnd.docker.system-events.v1+json-seq"

		headers.Add("Accept", types.MediaTypeJSONSequence)
		headers.Add("Accept", types.MediaTypeNDJSON)

Conversation

ndeloof commented Sep 11, 2025

Uh oh!

akerouanton commented Sep 11, 2025

Uh oh!

ndeloof commented Sep 11, 2025

Uh oh!

thaJeztah commented Sep 11, 2025

Uh oh!

ndeloof commented Sep 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ndeloof commented Sep 17, 2025

Uh oh!

thaJeztah commented Sep 19, 2025

application/json-seq (RFC7464)

Use Server-Sent Events (SSE) (text/event-stream)

Use application/x-ndjson

Uh oh!

thaJeztah commented Sep 19, 2025

Uh oh!

ndeloof commented Sep 19, 2025

Uh oh!

thaJeztah commented Sep 19, 2025

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thaJeztah commented Sep 23, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ndeloof Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Oct 3, 2025

Uh oh!

thaJeztah commented Oct 3, 2025

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

ndeloof commented Sep 13, 2025 •

edited

Loading

`application/json-seq` (RFC7464)

Use Server-Sent Events (SSE) (`text/event-stream`)

Use `application/x-ndjson`

ndeloof Sep 23, 2025 •

edited

Loading