Enhancement: Check ComfyUI server cache before image upload

[rbeurskens]

Should solve #2467
Also saves some network traffic by not sending the payload if HEAD returned 200. (in contrast to PUT)

[Acly]

I've never encountered the "Unable to write" error before, does that happen all the time for you or with particular image size? Which OS?

I don't think an additional HEAD request is a necessarily good trade of, now every upload needs an extra round trip, and the not-cached case is probably far more common.

I consider sending some of the upload data even if it's cached okay, but it would require a way to selectively ignore this particular error, which might be difficult if Qt doesn't return the actual response and 200 status code. Since I can't reproduce it, it's hard to tell.

Cleaner solution might be to start the PUT with a Expect: 100-continue header which is less overhead than the HEAD request, but I haven't tested it yet.

[rbeurskens]

For me, consistent from consecutive prompts using one or more of the same source image(s) when using a custom ComfyUI server (latest) running on Kubuntu LTS 24.04 on a 2012 era PC (MSI Z77 GD65 mainboard, i7 3770K, 16GB). Krita running on Windows 10 12gen i7,16GB.
I will test your suggestion and let you know if that fixes it too.
The issue could be caused by the combination of the modern Linux running on 2012 hardware (in particular when it comes to power management of the onboard ethernet adapter)

Images were not really large (this one was ~1MB )

[Acly]

Might just have to do with latency, I mostly test local with Krita & Comfy running on the same machine.

Note that the 100-continue solution would require support by the server too (ie. in comfyui-tooling-nodes)

[rbeurskens]

Yes, probably. I never had this when running the local managed server. After all, the loopback likely doesn't have this issue. But with an actual remote it's different. (maybe even worse with a cloud server instead of one in the LAN)
I have the latest comfyUI-tooling-nodes node cloned on the server. As far as I know the header is a high level protocol feature that does not need hardware/driver level support to work, as long as the HTTP listener supports it. (aiohttp)

The problem is that, as you also said, QtReply will only process response data if it sent the full payload successfully. No matter if the PUT is sent with Expect-100 or without, if the server sees it has the image cached before the client has finished sending the payload, it will send the 200 OK response with the { "status" : "cached" } json and close the connection immediately. This will cause QNetworkReply::RemoteHostClosedError.
If the lightweight HEAD it too much, we would need to handle the request on a lower level and handle the request manually using QTcpSocket, and keep monitoring server response for 200 OK and stop once it's found to prevent the error QNetworkAccessManager would trigger.

[Acly]

Not sure why you think it needs to be handled at a lower level, the 100-Continue mechanism is just HTTP.

As far as I understand you need to change the client to send Expect-100 in header, change the server to respond with 100-Continue if uncached or 200 if cached, and the client to send the data when it receives 100 response.

I'm not 100% sure it can be done with QNetworkManager, if it's not possible then I'd prefer to just silence the "Unable to write" error.

[rbeurskens]

Because 100 Continue does not solve the issue. (curl already sends this header under the hood, and as you may see in the output in my example, it still results in a disconnect before the client can send the payload.) In the high level method with QNetworkReply, I tried hooking in a handle error method and handle any QNetworkReply::RemoteHostClosedError, but the socket is aborted by the server and it is a known behavior if the response does not return with a 'success', QNetworkReply does not offer a way to read any response headers or data (it's like an 'all-or-nothing' black box.) , so I can't check the content-length, content-type and the content itself to verify if despite the disconnect the server's response is complete in order to know if we are dealing with a real error or a cache hit.
And if we need to rely on altering the server (in this case ConfyUI and we may not always have access to the server backend) in its response behavior, that doesn't sound like a very flexible solution.

[rbeurskens]

A bit more context of what I tried before: I added an error handler in network.py in the http() method:

        def debug_log(err):
            status = reply.attribute(QNetworkRequest.HttpStatusCodeAttribute)
            log.info(f"DEBUG: Error {err} occurred. Status: {status}")

        reply.errorOccurred.connect(debug_log)

This resulted in the following: (note status is None, not 100 or 200)
Error 99 is QNetworkReply.UnknownNetworkError

2026-04-26 08:06:56,581 INFO DEBUG: Error 99 occurred. Status: None
2026-04-26 08:06:56,612 ERROR Error uploading image 1d59b868: Unable to write
2026-04-26 08:06:56,612 ERROR Unhandled exception while processing Job[id=6e7d7e08-55a2-4e7e-970f-78fb0828dc58]
Traceback (most recent call last):
  File "%AppData%\Roaming\krita\pykrita\ai_diffusion\comfy_client.py", line 555, in upload_images
    await self._put(f"api/etn/image/{id}", data2, timeout=120)
  File "%AppData%\Roaming\krita\pykrita\ai_diffusion\comfy_client.py", line 280, in _put
    return await self._requests.put(f"{self.url}/{op}", data, timeout=timeout)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ai_diffusion.network.NetworkError: Unable to write

[Acly]

Yes it absolutely needs server side support! My hope was it can just be added here: https://github.com/Acly/comfyui-tooling-nodes/blob/main/api.py#L351-L366

This code is maintained by me so we can change it without modifying ComfyUI.

[rbeurskens]

Ah, cool. That is good to know. Not sure if it would help dealing with how the disconnect is handled, as I think this is likely a problem on the underlying netwerk/adapter/driver, but it can open more options to implement a solution.

We could modify it something like this: (untested draft)

    @_server.routes.put("/api/etn/image/{id}")
    async def put_image(request: web.Request):
        try:
            id = request.match_info.get("id", "")
            if id in image_cache:
                # If client sent Expect: 100-continue, this 200 OK 
                # tells it: "Don't send the data."
                # request.release() ensures any stray bytes are handled.
                await request.release() 
                return web.json_response(dict(status="cached"), status=200)

            # Accessing request.content now triggers the '100 Continue' 
            # signal to the client to start uploading.
            content_type = request.headers.get("Content-Type", "application/octet-stream")
            data = bytearray()
            async for chunk, _ in request.content.iter_chunks():
                data.extend(chunk)

            image_cache.insert(id, bytes(data), content_type)
            return web.json_response(dict(status="success"), status=201)
        except Exception as e:
            # If upload fails mid-way, request.release() helps clean up
            await request.release()
            return web.json_response(dict(error=str(e)), status=500)

.. and modify the client code to make the decision based on the accept-100 to proceed uploading as you suggested earlier:

    def http(
        self,
        method: str,
        url: str,
        data: dict | QByteArray | None = None,
        timeout: float | None = None,
        bearer: str | None = None,
    ):
        self._cleanup()

        request = self._prepare_request(url, timeout, bearer)

        assert method in ["GET", "POST", "PUT", "HEAD"]
        if method == "POST":
            # (...)
        elif method == "PUT":
            if isinstance(data, bytes):
                data = QByteArray(data)
            assert isinstance(data, QByteArray)
            
            # --- Handshake Optimization ---
            # Tell the server we have a payload but want to check headers first.
            # This prevents Error 99/Socket Error 10 by allowing the server 
            # to return "cached" before the client starts the upload.
            request.setRawHeader(b"Expect", b"100-continue")
            
            request.setHeader(
                QNetworkRequest.KnownHeaders.ContentTypeHeader, "application/octet-stream"
            )
            request.setHeader(QNetworkRequest.KnownHeaders.ContentLengthHeader, data.size())
            reply = self._net.put(request, data)
        else:
            reply = self._net.get(request)
     
        future = asyncio.get_running_loop().create_future()
        self._requests[reply] = Request(url, future)
        return future

[Acly]

This is what I mean: Acly/comfyui-tooling-nodes#65

I don't have time to fully test right now, maybe you can check if it works. I can review/merge later.

[rbeurskens]

Sure I can test it as I have the environment to reproduce the problem. I will let you know.

[Acly]

I merged the server-side changes. This should already help, but I'm not sure it uses the efficient path.

Did you ever check if the client sends the Expect 100 header already, or if it's easy to make it do that (without low-level changes)?

[rbeurskens]

I merged the server-side changes. This should already help, but I'm not sure it uses the efficient path.

Did you ever check if the client sends the Expect 100 header already, or if it's easy to make it do that (without low-level changes)?

I tested it with repeated uploads with curl. And this already looks like how it should look :

PS>> curl.exe -v -X PUT --data-binary "@my_image.jpg" http://<myserver>:8188/api/etn/image/1d59b868
*   Trying <myserver>:8188...
* Connected to <myserver> (<server_ip>) port 8188
* using HTTP/1.x
> PUT /api/etn/image/1d59b868 HTTP/1.1
> Host: <myserver>:8188
> User-Agent: curl/8.13.0
> Accept: */*
> Content-Length: 1067265
> Content-Type: application/x-www-form-urlencoded
> Expect: 100-continue
>
* Done waiting for 100-continue
* upload completely sent off: 1067265 bytes
< HTTP/1.1 201 Created
< Content-Type: application/json; charset=utf-8
< Content-Length: 21
< Date: Thu, 14 May 2026 01:09:47 GMT
< Server: Python/3.12 aiohttp/3.13.5
<
{"status": "success"}* Connection #0 to host <myserver> left intact

PS>> curl.exe -v -X PUT --data-binary "@my_image.jpg" http://<myserver>:8188/api/etn/image/1d59b868
*   Trying <myserver>:8188...
* Connected to <myserver> (<server_ip>) port 8188
* using HTTP/1.x
> PUT /api/etn/image/1d59b868 HTTP/1.1
> Host: <myserver>:8188
> User-Agent: curl/8.13.0
> Accept: */*
> Content-Length: 1067265
> Content-Type: application/x-www-form-urlencoded
> Expect: 100-continue
>
< HTTP/1.1 200 OK
< Content-Type: application/json; charset=utf-8
< Content-Length: 20
< Date: Thu, 14 May 2026 01:10:42 GMT
< Server: Python/3.12 aiohttp/3.13.5
<
{"status": "cached"}* Connection #0 to host <myserver> left intact

So server side should be good. I'll modify the client code in this PR to handle it like curl does. Indeed no low level code should be needed indeed, I expect. Likely a very small change.

[rbeurskens]

I have tested it with the small adjustment I thought that would work (just adding the header), but it seems Qt5 has no support for this workflow, and simply ignores it. (if I use a QtBuffer instead of a bytearray, which should allow it to wait for the server response, the entire process crashes with QtNetworkManager.)
From what I found this is a Qt5 issue, and since we are stuck to it because Krita uses it (not Qt6, where it might be fixed), we unfortunately still need a QTcpSocket approach when sending this header to properly handle this (default) http workflow.

WIP...

[Acly]

Nevermind then, if it's not possible with Qt's HTTP features I don't think we want it. Reimplementing this with sockets is not worth the complexity.

Qt6 migration is imminent, not sure if it will help but worth checking out.

[rbeurskens]

I'm implementing it for myself anyway, and it will be easy to toggle it only for this workflow. I'll share it when finished. I'm already testing it. (it's not even certain Qt6 will fix it)