Nodejs-storage: Simple file upload doesn't respect apiEndpoint

Created on 24 Oct 2019  路  9Comments  路  Source: googleapis/nodejs-storage

File uploading does not respect apiEndpoint or the environment variable STORAGE_EMULATOR_HOST. Instead it uses a hardcoded constant STORAGE_UPLOAD_BASE_URL

Environment details

  • OS: Mac 10.14.6
  • Node.js version: 12.12.0
  • npm version: 6.11.3
  • @google-cloud/storage version: 4.0.0
storage p2 bug

Most helpful comment

@frankyn let me know if you think otherwise:

I think we should keep upload requests at the /upload endpoint, as-per GCS documentation. For the particular use case of fake-gcs-server by @chrisrecalis, this endpoint is supported.

+1 to STORAGE_EMULATOR_HOST mapping to options.apiEndpoint, question is which one should take precedence?

All 9 comments

After a little investigation I can see why it was done this way:

In @google-cloud/common the ServiceObject function request_ builds a uri based on the object id and its parent id. Unless an absolute URL is provided. If the absolute URL wasn't provided in
https://github.com/googleapis/nodejs-storage/blob/f2d567f279fee0f48c2d6a607f4066c9da372549/src/file.ts#L3398-L3403

The request uri would build to be something like https://{apiEndpoint}/storage/v1/b/my-bucket/o/myfile.txt when trying to upload a file myfile.txt. Which doesn't conform to https://cloud.google.com/storage/docs/json_api/v1/objects/insert.

I've since just used an interceptor instead:

const storage = new Storage({
    projectId: 'test'
});

storage.interceptors.push({
    request: function(reqOpts) {
        const url = new URL(reqOpts.uri)
        url.host = "127.0.0.1:4443"
        reqOpts.uri = url.toString()
        return reqOpts
    }
})

Thanks for digging into that, and sharing the solution you found! Is STORAGE_EMULATOR_HOST an official env var? That follows the format from Datastore, although unlike Datastore, I haven't found an official Storage API emulator.

@frankyn what is the extent of support we want to provide for this use case?

I'm currently using fake-gcs-server as my emulator. The environment variable was dug up in source code: https://github.com/googleapis/nodejs-storage/blob/f2d567f279fee0f48c2d6a607f4066c9da372549/src/storage.ts#L375-L379

Would it work if it followed the same creation method as Buckets? ie the bucket.create() method calls the Service.createBucket method. If a File called Bucket.createFile(aka startSimpleUpload_) then when the uri is built inside Bucket.request({ uri: '/o' }) it would end up building the correct url.

@stephenplusplus, STORAGE_EMULATOR_HOST env var was introduced for benchmarking by @jadekler. We don't have an official GCS emulator at the moment.

IIUC, STORAGE_EMULATOR_HOST should have the same effect as setting apiEndpoint.

Ah, nice, I didn't know that slipped in there. I think we can figure this out. I'll assign to myself and update everyone accordingly for reviews!

@frankyn In our other APIs, options.apiEndpoint and the corresponding emulator environment variable are expected to have the same side effects when set-- everything is appended to that "base" URL.

Currently in Storage, however, the emulator environment variable replaces https://storage.googleapis.com/storage/v1, while options.apiEndpoint only replaces storage.googleapis.com.

Since options.apiEndpoint was designed for the same purpose as the environment variable and STORAGE_EMULATOR_HOST isn't an official environment variable, can we rethink this to have both behave equally?

In any case, what would be the correct URL to send the upload request to:

  1. https://${userProvidedOverride}/upload/storage/v1/b/${this.bucket.name}/o
  2. https://${userProvidedOverride}/b/${this.bucket.name}/o (the /upload/ prefix removed, otherwise /storage/v1/ is duplicated)

Hope this made sense!

@frankyn let me know if you think otherwise:

I think we should keep upload requests at the /upload endpoint, as-per GCS documentation. For the particular use case of fake-gcs-server by @chrisrecalis, this endpoint is supported.

+1 to STORAGE_EMULATOR_HOST mapping to options.apiEndpoint, question is which one should take precedence?

If this can help anyone, the workaround provided will not work with:
File.createWriteStream() as it uses startResumableUpload_ instead of startSimpleUpload_.
You can use File.createWriteStream({resumable: false}) in the meantime.

Was this page helpful?
0 / 5 - 0 ratings