Stop reporting stale replies as successful conversions

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- Report the real error when a conversion fails at the socket level, instead of returning the
+  stale interface OID left in the reply as if it were the converted output.
+
 ## [v0.10.2] - 2026-07-14
 
 ### Changed

lib/urp/pool.ex

@@ -108,30 +108,48 @@ defmodule URP.Pool do
         |> load_input(input, io_in)
         |> store_output(store_opts, sink, io_out)
 
+      # Bridge.cleanup/1 only ever appends to conn.error, so this is the last
+      # point at which we can tell a failed conversion from a failed cleanup.
+      convert_error = conn.error
       result = conn.reply
       conn = Bridge.cleanup(conn)
       conn = %{conn | max_frame_size: default_max_frame_size, recv_timeout: default_recv_timeout}
 
-      # Stream-based input registers an XInputStream at a fixed OID cache slot.
-      # soffice's URP cache doesn't fully reset on reuse, producing truncated
-      # documents on subsequent stream loads. Discard the connection to force
-      # a fresh handshake. File-based I/O reuses connections normally.
-      reusable = not stream_input? and is_nil(conn.error)
-      has_result = is_binary(result) or result == :ok
+      case checkout_outcome(result, convert_error, conn.error, stream_input?) do
+        {value, :reuse} -> {value, {:ok, reset_conversion_state(conn)}}
+        {value, :discard} -> {value, :closed}
+      end
+    end)
+  end
+
+  @doc false
+  @spec checkout_outcome(term(), String.t() | nil, String.t() | nil, boolean()) ::
+          {:ok | {:ok, binary()} | {:error, String.t()}, :reuse | :discard}
+  def checkout_outcome(result, convert_error, error, stream_input?) do
+    # Stream-based input registers an XInputStream at a fixed OID cache slot.
+    # soffice's URP cache doesn't fully reset on reuse, producing truncated
+    # documents on subsequent stream loads. Discard the connection to force
+    # a fresh handshake. File-based I/O reuses connections normally.
+    reusable = not stream_input? and is_nil(error)
+
+    # A socket-level failure leaves conn.reply holding whatever the last call
+    # parsed — typically an interface OID — so the shape of the reply alone
+    # cannot tell output from leftovers. Only trust it if the conversion itself
+    # reported no error.
+    has_result = is_nil(convert_error) and (is_binary(result) or result == :ok)
 
-      cond do
-        reusable ->
-          {wrap_result(result), {:ok, reset_conversion_state(conn)}}
+    cond do
+      reusable ->
+        {wrap_result(result), :reuse}
 
-        has_result ->
-          # Conversion succeeded but close/cleanup failed (e.g. soffice drops
-          # the connection after stream→stream). Return the result, discard conn.
-          {wrap_result(result), :closed}
+      has_result ->
+        # Conversion succeeded but close/cleanup failed (e.g. soffice drops
+        # the connection after stream→stream). Return the result, discard conn.
+        {wrap_result(result), :discard}
 
-        true ->
-          {{:error, conn.error}, :closed}
-      end
-    end)
+      true ->
+        {{:error, error}, :discard}
+    end
   end
 
   defp normalize_io(:file), do: {:file, :file}

mix.lock

@@ -12,7 +12,7 @@
   "makeup_elixir": {:hex, :makeup_elixir, "1.0.1", "e928a4f984e795e41e3abd27bfc09f51db16ab8ba1aebdba2b3a575437efafc2", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "7284900d412a3e5cfd97fdaed4f5ed389b8f2b4cb49efc0eb3bd10e2febf9507"},
   "makeup_erlang": {:hex, :makeup_erlang, "1.1.0", "835f7e60792e08824cda445639555d7bf1bbbddb1b60b306e33cb6f6db24dc74", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "1cd6780fb1dd1a03979abaed0fe82712b0625118fd5257d3ebbf73f960c73c3c"},
   "mime": {:hex, :mime, "2.0.7", "b8d739037be7cd402aee1ba0306edfdef982687ee7e9859bee6198c1e7e2f128", [:mix], [], "hexpm", "6171188e399ee16023ffc5b76ce445eb6d9672e2e241d2df6050f3c771e80ccd"},
-  "mint": {:hex, :mint, "1.9.2", "6e89e698d69cc29be001afd02c2cf4bae2d0994efe69a2ced7aab440d71584f9", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:hpax, "~> 0.1.1 or ~> 0.2.0 or ~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}], "hexpm", "d8e952b432fdac2321f570d29a68b9eb2664dc80a7a2ef9464531a5425abf222"},
+  "mint": {:hex, :mint, "1.9.3", "3337184d69179695c7a9f1714d92c11e629d36c8c037a21cf490131d3d150554", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:hpax, "~> 0.1.1 or ~> 0.2.0 or ~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}], "hexpm", "5f7c9342480c069dbbc4eeac3490303c9e01870ff01a7f1d29b6107054fc1e74"},
   "nimble_options": {:hex, :nimble_options, "1.1.1", "e3a492d54d85fc3fd7c5baf411d9d2852922f66e69476317787a7b2bb000a61b", [:mix], [], "hexpm", "821b2470ca9442c4b6984882fe9bb0389371b8ddec4d45a9504f00a66f650b44"},
   "nimble_ownership": {:hex, :nimble_ownership, "1.0.2", "fa8a6f2d8c592ad4d79b2ca617473c6aefd5869abfa02563a77682038bf916cf", [:mix], [], "hexpm", "098af64e1f6f8609c6672127cfe9e9590a5d3fcdd82bc17a377b8692fd81a879"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.4.2", "8efba0122db06df95bfaa78f791344a89352ba04baedd3849593bfce4d0dc1c6", [:mix], [], "hexpm", "4b21398942dda052b403bbe1da991ccd03a053668d147d53fb8c4e0efe09c973"},

test/urp/pool_test.exs

@@ -0,0 +1,45 @@
+defmodule URP.PoolTest do
+  use ExUnit.Case, async: true
+
+  # Real OID observed in production when soffice aborted mid-conversion.
+  @stale_oid "57ee30e3f490;gcc3[0];46d08adf17bf43b189511ea4ed49b46f"
+
+  describe "checkout_outcome/4" do
+    test "keeps the connection when a sink consumed the output" do
+      assert URP.Pool.checkout_outcome(:ok, nil, nil, false) == {:ok, :reuse}
+    end
+
+    test "wraps the output bytes when no sink consumed them" do
+      assert URP.Pool.checkout_outcome("%PDF-1.7", nil, nil, false) ==
+               {{:ok, "%PDF-1.7"}, :reuse}
+    end
+
+    test "discards the connection after stream input even on success" do
+      assert URP.Pool.checkout_outcome(:ok, nil, nil, true) == {:ok, :discard}
+    end
+
+    test "returns the result but discards the connection when only cleanup failed" do
+      assert URP.Pool.checkout_outcome(:ok, nil, "close failed", false) == {:ok, :discard}
+    end
+
+    test "reports the error when the conversion produced nothing" do
+      assert URP.Pool.checkout_outcome(nil, "connection closed", "connection closed", false) ==
+               {{:error, "connection closed"}, :discard}
+    end
+
+    test "reports the error when a failed conversion left a stale OID in the reply" do
+      assert URP.Pool.checkout_outcome(
+               @stale_oid,
+               "connection closed",
+               "connection closed",
+               false
+             ) ==
+               {{:error, "connection closed"}, :discard}
+    end
+
+    test "reports the error when a failed conversion left stale bytes in the reply" do
+      assert URP.Pool.checkout_outcome(<<0x80, 0, 0, 0, 0>>, "timeout", "timeout", true) ==
+               {{:error, "timeout"}, :discard}
+    end
+  end
+end