Skip to content

Fix Stream.closeWritable to await close completion - #133

Merged
robinheghan merged 1 commit into
gren-lang:mainfrom
dkoontz:stream-close-bug
Aug 3, 2026
Merged

Fix Stream.closeWritable to await close completion#133
robinheghan merged 1 commit into
gren-lang:mainfrom
dkoontz:stream-close-bug

Conversation

@dkoontz

@dkoontz dkoontz commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Fix Stream.closeWritable to await close completion and surface errors

Problem

_Stream_closeWritable in src/Gren/Kernel/Stream.js was fire-and-forget:

const writer = stream.getWriter();
writer.close();          // promise ignored
writer.releaseLock();
callback(__Scheduler_succeed({}));

writer.close() returns a promise that settles only once the underlying sink has actually closed. By ignoring it, the function had two correctness bugs:

  1. Premature success. The Task resolved synchronously — before the stream had actually closed. Any caller chaining on the result (Task.andThen) would proceed as though the stream were closed when it wasn't yet.
  2. Silently swallowed errors. If closing failed (the sink rejected, or the stream errored mid-close), the writer.close() promise rejected with no listener. The unhandled rejection was lost, and the Task still reported success — callers could never learn the close failed.

Fix

Await writer.close() and route its outcome to the Scheduler callback:

writer
  .close()
  .then(() => {
    writer.releaseLock();
    callback(__Scheduler_succeed({}));
  })
  .catch((err) => {
    writer.releaseLock();
    callback(
      __Scheduler_fail(
        __Stream_Cancelled(_Stream_cancellationErrorString(err)),
      ),
    );
  });

The .catch path reuses the existing _Stream_cancellationErrorString helper, matching how _Stream_read already reports failures as Stream.Cancelled.

Behavior changes

  • closeWritable now resolves only after the stream has actually closed, so chained tasks observe correct sequencing.
  • Close failures are now surfaced as Stream.Cancelled <message> instead of being swallowed and falsely reported as success.
  • The writer lock is now released after close() settles (in both branches), rather than synchronously before close completes.

@robinheghan
robinheghan merged commit 9818fae into gren-lang:main Aug 3, 2026
1 check failed
@robinheghan

Copy link
Copy Markdown
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants