Fix handling of deferred write errors that happen when closing

10 files changed, 77 insertions, 30 deletions

diff --git a/include/serd/serd.h b/include/serd/serd.h
index d606f4bb..35d31a57 100644
--- a/include/serd/serd.h
+++ b/include/serd/serd.h
@@ -2201,8 +2201,8 @@ serd_node_to_syntax(const SerdNode* SERD_NONNULL node,
 /// An input stream that produces bytes
 typedef struct {
   void* SERD_NULLABLE         stream; ///< Opaque parameter for functions
-  SerdReadFunc SERD_NULLABLE  read;   ///< Read bytes to input
-  SerdErrorFunc SERD_NONNULL  error;  ///< Stream error accessor
+  SerdReadFunc SERD_NONNULL   read;   ///< Read bytes from input
+  SerdErrorFunc SERD_NULLABLE error;  ///< Stream error accessor
   SerdCloseFunc SERD_NULLABLE close;  ///< Close input
 } SerdInputStream;
 
@@ -2468,7 +2468,8 @@ serd_buffer_close(void* SERD_NONNULL stream);
 /// An output stream that receives bytes
 typedef struct {
   void* SERD_NULLABLE         stream; ///< Opaque parameter for functions
-  SerdWriteFunc SERD_NULLABLE write;  ///< Write bytes to output
+  SerdWriteFunc SERD_NONNULL  write;  ///< Write bytes to output
+  SerdErrorFunc SERD_NULLABLE error;  ///< Stream error accessor
   SerdCloseFunc SERD_NULLABLE close;  ///< Close output
 } SerdOutputStream;
 
@@ -2476,6 +2477,7 @@ typedef struct {
    Open a stream that writes to a provided function.
 
    @param write_func Function to write output.
+   @param error_func Function used to detect errors.
    @param close_func Function to close the stream after writing is done.
    @param stream Opaque stream parameter for write_func and close_func.
 
@@ -2484,6 +2486,7 @@ typedef struct {
 SERD_CONST_API
 SerdOutputStream
 serd_open_output_stream(SerdWriteFunc SERD_NONNULL  write_func,
+                        SerdErrorFunc SERD_NULLABLE error_func,
                         SerdCloseFunc SERD_NULLABLE close_func,
                         void* SERD_NULLABLE         stream);
 
@@ -2519,7 +2522,8 @@ serd_open_output_file(const char* SERD_NONNULL path);
 
    This will call the close function, and reset the stream internally so that
    no further writes can be made.  For convenience, this is safe to call on
-   NULL, and safe to call several times on the same output.
+   NULL, and safe to call several times on the same output.  Failure is
+   returned in both of those cases.
 */
 SERD_API
 SerdStatus
diff --git a/src/output_stream.c b/src/output_stream.c
index b98c834a..54397735 100644
--- a/src/output_stream.c
+++ b/src/output_stream.c
@@ -21,6 +21,7 @@
 // IWYU pragma: no_include <features.h>
 
 #include <assert.h>
+#include <stdbool.h>
 #include <stddef.h>
 #include <stdio.h>
 
@@ -30,12 +31,13 @@
 
 SerdOutputStream
 serd_open_output_stream(SerdWriteFunc const write_func,
+                        SerdErrorFunc const error_func,
                         SerdCloseFunc const close_func,
                         void* const         stream)
 {
   assert(write_func);
 
-  SerdOutputStream output = {stream, write_func, close_func};
+  SerdOutputStream output = {stream, write_func, error_func, close_func};
   return output;
 }
 
@@ -44,7 +46,8 @@ serd_open_output_buffer(SerdBuffer* const buffer)
 {
   assert(buffer);
 
-  return serd_open_output_stream(serd_buffer_write, serd_buffer_close, buffer);
+  return serd_open_output_stream(
+    serd_buffer_write, NULL, serd_buffer_close, buffer);
 }
 
 SerdOutputStream
@@ -59,7 +62,7 @@ serd_open_output_file(const char* const path)
 #endif
 
   if (!file) {
-    const SerdOutputStream failure = {NULL, NULL, NULL};
+    const SerdOutputStream failure = {NULL, NULL, NULL, NULL};
     return failure;
   }
 
@@ -68,22 +71,20 @@ serd_open_output_file(const char* const path)
 #endif
 
   return serd_open_output_stream(
-    (SerdWriteFunc)fwrite, (SerdCloseFunc)fclose, file);
+    (SerdWriteFunc)fwrite, (SerdErrorFunc)ferror, (SerdCloseFunc)fclose, file);
 }
 
 SerdStatus
 serd_close_output(SerdOutputStream* const output)
 {
-  int ret = 0;
+  if (!output || !output->stream) {
+    return SERD_FAILURE;
+  }
 
-  if (output) {
-    if (output->close && output->stream) {
-      ret            = output->close(output->stream);
-      output->stream = NULL;
-    }
+  const bool had_error = output->error ? output->error(output->stream) : false;
+  int        close_st  = output->close ? output->close(output->stream) : 0;
 
-    output->stream = NULL;
-  }
+  output->stream = NULL;
 
-  return ret ? SERD_BAD_WRITE : SERD_SUCCESS;
+  return (had_error || close_st) ? SERD_BAD_WRITE : SERD_SUCCESS;
 }
diff --git a/test/meson.build b/test/meson.build
index ae66ba37..a25ac6de 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -52,6 +52,7 @@ if autoship.found()
 endif
 
 serd_ttl = files('../serd.ttl')[0]
+small_ttl = files('../test/TurtleTests/turtle-syntax-number-01.ttl')[0]
 common_script_options = []
 if wrapper != ''
   common_script_options = ['--wrapper', wrapper]
@@ -230,6 +231,11 @@ if is_variable('serd_pipe')
        env: test_env,
        suite: ['tools', 'pipe', 'output'])
 
+  test('deferred_write_error', files('test_deferred_write_error.py'),
+       args: pipe_test_script_args + [small_ttl],
+       env: test_env,
+       suite: ['tools', 'pipe', 'output'])
+
   test('missing_output',
        tool,
        args: ['-o', '/does/not/exist.ttl', meson.source_root() / 'serd.ttl'],
diff --git a/test/test_deferred_write_error.py b/test/test_deferred_write_error.py
new file mode 100644
index 00000000..4005d570
--- /dev/null
+++ b/test/test_deferred_write_error.py
@@ -0,0 +1,31 @@
+#!/usr/bin/env python3
+
+"""Test errors that only really happen when the output file is closed."""
+
+import argparse
+import sys
+import shlex
+import subprocess
+import os
+
+parser = argparse.ArgumentParser(description=__doc__)
+
+parser.add_argument("--tool", default="tools/serd-pipe", help="executable")
+parser.add_argument("--wrapper", default="", help="executable wrapper")
+parser.add_argument("input", help="valid input file")
+
+args = parser.parse_args(sys.argv[1:])
+command = shlex.split(args.wrapper) + [args.tool, '-b', '1024', args.input]
+
+if os.path.exists("/dev/full"):
+
+    with open("/dev/full", "w") as out:
+        proc = subprocess.run(
+            command, check=False, stdout=out, stderr=subprocess.PIPE
+        )
+
+    assert proc.returncode != 0
+    assert "error" in proc.stderr.decode("utf-8")
+
+else:
+    sys.stderr.write("warning: /dev/full not present, skipping test")
diff --git a/test/test_model.c b/test/test_model.c
index 3e781c7e..a64a7b7e 100644
--- a/test/test_model.c
+++ b/test/test_model.c
@@ -1281,7 +1281,7 @@ test_write_error_in_list_subject(SerdWorld* world, const unsigned n_quads)
   for (size_t max_successes = 0; max_successes < 18; ++max_successes) {
     FailingWriteFuncState state = {0, max_successes};
     SerdOutputStream      out =
-      serd_open_output_stream(failing_write_func, NULL, &state);
+      serd_open_output_stream(failing_write_func, NULL, NULL, &state);
 
     SerdWriter* writer = serd_writer_new(world, SERD_TURTLE, 0, env, &out, 1);
 
@@ -1337,7 +1337,7 @@ test_write_error_in_list_object(SerdWorld* world, const unsigned n_quads)
   for (size_t max_successes = 0; max_successes < 21; ++max_successes) {
     FailingWriteFuncState state = {0, max_successes};
     SerdOutputStream      out =
-      serd_open_output_stream(failing_write_func, NULL, &state);
+      serd_open_output_stream(failing_write_func, NULL, NULL, &state);
 
     SerdWriter* writer = serd_writer_new(world, SERD_TURTLE, 0, env, &out, 1);
 
diff --git a/test/test_writer.c b/test/test_writer.c
index c719642f..6ffe5d0f 100644
--- a/test/test_writer.c
+++ b/test/test_writer.c
@@ -126,10 +126,12 @@ null_sink(const void* const buf,
 static void
 test_writer_stack_overflow(void)
 {
-  SerdWorld*       world  = serd_world_new();
-  SerdNodes*       nodes  = serd_world_nodes(world);
-  SerdEnv*         env    = serd_env_new(SERD_EMPTY_STRING());
-  SerdOutputStream output = serd_open_output_stream(null_sink, NULL, NULL);
+  SerdWorld* world = serd_world_new();
+  SerdNodes* nodes = serd_world_nodes(world);
+  SerdEnv*   env   = serd_env_new(SERD_EMPTY_STRING());
+
+  SerdOutputStream output =
+    serd_open_output_stream(null_sink, NULL, NULL, NULL);
 
   SerdWriter* writer = serd_writer_new(world, SERD_TURTLE, 0u, env, &output, 1);
 
@@ -244,7 +246,8 @@ test_write_error(void)
 
   // Test with setting errno
 
-  SerdOutputStream output = serd_open_output_stream(faulty_sink, NULL, NULL);
+  SerdOutputStream output =
+    serd_open_output_stream(faulty_sink, NULL, NULL, NULL);
 
   SerdWriter* writer = serd_writer_new(world, SERD_TURTLE, 0u, env, &output, 1);
   assert(writer);
@@ -256,7 +259,7 @@ test_write_error(void)
   serd_close_output(&output);
 
   // Test without setting errno
-  output = serd_open_output_stream(faulty_sink, NULL, world);
+  output = serd_open_output_stream(faulty_sink, NULL, NULL, world);
   writer = serd_writer_new(world, SERD_TURTLE, 0u, env, &output, 1);
 
   assert(writer);
diff --git a/tools/console.c b/tools/console.c
index 3b209273..c6e55c17 100644
--- a/tools/console.c
+++ b/tools/console.c
@@ -372,8 +372,10 @@ serd_open_tool_output(const char* const filename)
 {
   if (!filename || !strcmp(filename, "-")) {
     serd_set_stream_utf8_mode(stdout);
-    return serd_open_output_stream(
-      (SerdWriteFunc)fwrite, (SerdCloseFunc)fclose, stdout);
+    return serd_open_output_stream((SerdWriteFunc)fwrite,
+                                   (SerdErrorFunc)ferror,
+                                   (SerdCloseFunc)fclose,
+                                   stdout);
   }
 
   return serd_open_output_file(filename);
diff --git a/tools/serd-filter.c b/tools/serd-filter.c
index 0804d6ba..b172d05d 100644
--- a/tools/serd-filter.c
+++ b/tools/serd-filter.c
@@ -127,7 +127,7 @@ log_error(SerdWorld* const world, const char* const fmt, ...)
 static SerdStatus
 run(Options opts)
 {
-  SerdTool app = {{NULL, NULL, NULL}, NULL, NULL, NULL};
+  SerdTool app = {{NULL, NULL, NULL, NULL}, NULL, NULL, NULL};
 
   // Set up the writing environment
   SerdStatus st = SERD_SUCCESS;
diff --git a/tools/serd-pipe.c b/tools/serd-pipe.c
index f2891f9b..612826ca 100644
--- a/tools/serd-pipe.c
+++ b/tools/serd-pipe.c
@@ -39,7 +39,7 @@ typedef struct {
 static SerdStatus
 run(const Options opts)
 {
-  SerdTool app = {{NULL, NULL, NULL}, NULL, NULL, NULL};
+  SerdTool app = {{NULL, NULL, NULL, NULL}, NULL, NULL, NULL};
 
   // Set up the writing environment
   SerdStatus st = SERD_SUCCESS;
diff --git a/tools/serd-sort.c b/tools/serd-sort.c
index 7219fa26..267e75f8 100644
--- a/tools/serd-sort.c
+++ b/tools/serd-sort.c
@@ -57,7 +57,7 @@ input_has_graphs(const Options opts)
 static SerdStatus
 run(const Options opts)
 {
-  SerdTool app = {{NULL, NULL, NULL}, NULL, NULL, NULL};
+  SerdTool app = {{NULL, NULL, NULL, NULL}, NULL, NULL, NULL};
 
   // Set up the writing environment
   SerdStatus st = SERD_SUCCESS;