ByteString.Output is missing an override for write(byte[]) that does not throw IOException

[archiecobbs]

What language does this apply to?

Java

Describe the problem you are trying to solve.

The ByteString.Output class extends OutputStream. Because it actually writes into memory, not into some I/O stream, the write(int b) and write(byte[] a, int off, int len) methods are declared to not throw IOException - which is a great feature!

However, this feature has an omission, which is that the write(byte[] a) method is not included in this feature.

Describe the solution you'd like

Simply have ByteString.Output declare an override for write(byte[] a) that does not throw IOException:

diff --git a/java/core/src/main/java/com/google/protobuf/ByteString.java b/java/core/src/main/java/com/google/protobuf/ByteString.java
index 1903a4deb..d8d6ebd2b 100644
--- a/java/core/src/main/java/com/google/protobuf/ByteString.java
+++ b/java/core/src/main/java/com/google/protobuf/ByteString.java
@@ -1088,6 +1088,11 @@ public abstract class ByteString implements Iterable<Byte>, Serializable {
       buffer[bufferPos++] = (byte) b;
     }
 
+    @Override
+    public void write(byte[] b) {
+        this.write(b, 0, b.length);
+    }
+
     @Override
     public synchronized void write(byte[] b, int offset, int length) {
       if (length <= buffer.length - bufferPos) {

Describe alternatives you've considered

Uglying up my code by catching and ignoring an IOException that will never be thrown.

[zhangskz]

Unfortunately while this makes a lot of sense, this would break anyone currently using writeTo and catching the exception, and is likely not worth the disruption / breaking change.

But please reopen if there's something I've failed to consider here that would make this more doable!

error: exception IOException is never thrown in body of corresponding try statement
    } catch (IOException e) {

[archiecobbs]

Unfortunately while this makes a lot of sense, this would break anyone currently using writeTo and catching the exception, and is likely not worth the disruption / breaking change.

Agree, it's not appropriate for a minor release for that reason, but perhaps would be for the next major release. Even though a new error would occur, it would only do so in situations where a code clean up is possible, which hopefully people would appreciate.

But please reopen if there's something I've failed to consider here that would make this more doable!

What about a non-overriding version of the method then? E.g.:

diff --git a/java/core/src/main/java/com/google/protobuf/ByteString.java b/java/core/src/main/java/com/google/protobuf/ByteString.java
index 1903a4deb..d8d6ebd2b 100644
--- a/java/core/src/main/java/com/google/protobuf/ByteString.java
+++ b/java/core/src/main/java/com/google/protobuf/ByteString.java
@@ -1088,6 +1088,15 @@ public abstract class ByteString implements Iterable<Byte>, Serializable {
       buffer[bufferPos++] = (byte) b;
     }
 
+    /**
+     * Write the entire contents of the given array.
+     *
+     * @param b byte data to write
+     */
+    public void writeArray(byte[] b) {
+        this.write(b, 0, b.length);
+    }
+
     @Override
     public synchronized void write(byte[] b, int offset, int length) {
       if (length <= buffer.length - bufferPos) {

FYI I'm not permitted to reopen this. Thanks.

[zhangskz]

Our next breaking change in Java will likely be in ~2026 Q1 which is the earliest we could probably consider this. Unfortunately this would require some effort to clean-up since I'm not sure there's a great way to address this. Simply doing this in a breaking change would likely require users with many call-sites (incl. google) to update atomically.

I don't think we'd want to introduce a second API, unless it were temporary with a plan to delete it.

To do this properly, I think we would need to do something like:

1. Add writeWithoutException(byte[] b)
2. Users incrementally migrate to writeWithoutException, removing try/catch
3. Major release: Update write to stop declaring IOException and deprecate writeWithoutException
4. Users migrate call-sites from writeWithoutException to write
5. Major release: Remove writeWithoutException

This is a lot of disruption with not a ton of value for existing users unfortunately. :(

[archiecobbs]

OK, thanks.

I was hoping ByteString would be useful as a general purpose, thread-safe, immutable holder of byte[] data. It's close, but doesn't quite hit the mark (see also #19746 for example).

I'll look elsewhere.