-
Notifications
You must be signed in to change notification settings - Fork 368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
File.save() with a Uint8Array fails #2478
Comments
@aiden-jeffrey did you also update node versions as part of the move from 6.x to 7.x? Looking at the signature for If you updated Node versions perhaps there used to be an automatic conversion that was changed. |
No. I didn't change node version. We upgraded to |
If it isn't clear, Aiden and I work together. I also don't think 7.0.0 is the first bad commit. I checked out this repo, bisected the commits, and landed on c0d9d58 to be the commit that starts failing when passing in a EDIT: diff --git a/src/file.ts b/src/file.ts
index 6eed2ca..dc260a0 100644
--- a/src/file.ts
+++ b/src/file.ts
@@ -3679,7 +3679,7 @@ class File extends ServiceObject<File, FileMetadata> {
reject(err);
};
- if (typeof data === 'string' || Buffer.isBuffer(data)) {
+ if (typeof data === 'string' || Buffer.isBuffer(data) || data instanceof Uint8Array) {
writable
.on('error', handleError)
.on('finish', () => resolve()) |
@sidsethupathi beat me to it. I was just looking at those lines. It looks like Uint8Array worked previously because despite the TypeScript signature saying |
The interesting thing is that we are using TypeScript so we'll look into why TS didn't yell at us for trying to pass in a |
When I tried in TypeScript just now, it wouldn't accept a
|
Yes, we have some abstract classes on top of our GCS invocations so I think something got messed up there. In short, we have an abstract class abstract class Writer {
abstract saveBinary(data: Buffer | Uint8Array);
} and then a GCS specific implementation class GCSWriter extends Writer {
saveBinary(data: Buffer) {
// call File.save(data) here
}
} I think since EDIT: Where this all falls apart is that that we never call abstract class Writer {
abstract saveBinary(data: Buffer | Uint8Array);
transform(foo: Foo) {
const data: Uint8Array = convertFooToUint8Array(foo);
this.saveBinary(data);
}
}
const gcsWriter = new GCSWriter();
gcsWriter.transform(foo); |
@sidsethupathi thank you for your investigation. I've opened a PR to add support for Uint8Array. |
Thank you! I was sure I'd checked the type definition in |
When saving a UintArray8, we see the error
The "chunk" argument must be of type string or an instance of Buffer or Uint8Array.
. This bug is only present in 7.0.0 onwards. I have to cast it to a buffer for the save to succeed, which wasn't necessary on 6.x.x.Environment details
@google-cloud/storage
version: 7.11.1Steps to reproduce
The text was updated successfully, but these errors were encountered: