If the user provides "options.apiEndpoint", that should override the env var. Similar to options.keyFilename beating ADC.

@jkwlui

Let's document the format expected for the env var. It looks like it needs to be a full protocol://host/path, however in the following line, the protocol will be lost.

To compare, nodejs-datastore accepts 'https://localhost:8080' or 'localhost:8080'

https://github.com/googleapis/nodejs-datastore/blob/68790d26d69f48d780930f90be6758f7431f61c9/src/index.ts#L97
https://github.com/googleapis/nodejs-datastore/blob/68790d26d69f48d780930f90be6758f7431f61c9/src/index.ts#L866

Can/should we accept both here?

I'm not really sure what we want to do here -

So we have STORAGE_EMULATOR_HOST that provides the full URL to override baseUrl upon which all requests are based, except simple upload and resumable upload.

StorageOptions.apiEndpoint also does not override simple upload and resumable upload, since it only affects the baseUrl set, as far as I can tell.

Simple upload currently uses STOARGE_UPLOAD_BASE_URL which is hardcoded to https://storage.googleapis.com/upload/storage/v1/b.

Resumable upload currently uses https://${this.apiEndpoint}/upload/storage/v1/b, where apiEndpoint defaults to storage.googleapis.com, which is overridable through cfg.apiEndpoint passed to resumableUpload.createURI, but apiEndpoint doesn't seem to be passed.
https://github.com/googleapis/gcs-resumable-upload/blob/master/src/index.ts#L212
https://github.com/googleapis/nodejs-storage/blob/master/src/file.ts#L1520-L1536

Thank you for tracking all of those scenarios down. It is a bit messy, eh?

So we have STORAGE_EMULATOR_HOST that provides the full URL to override baseUrl upon which all requests are based, except simple upload and resumable upload.

AFAIK, STORAGE_EMULATOR_HOST is not an official, Google-made env var. So we get a bit of play here to define it ourselves.

We should have it also affect the simple and resumable paths.

StorageOptions.apiEndpoint also does not override simple upload and resumable upload, since it only affects the baseUrl set, as far as I can tell.

We should make sure it overrides it when apiEndpoint is set, too.

Simple upload currently uses [...] https://storage.googleapis.com/upload/storage/v1/b
Resumable upload currently uses https://${this.apiEndpoint}/upload/storage/v1/b

Let's just sync these up.

Overall, what I think would be least surprising for users:

For here-ish:

const url =
  sanitize(options.apiEndpoint) || 
  sanitize(process.env.STORAGE_EMULATOR_HOST) ||
  'https://storage.googleapis.com'

url += '/storage/v1'

function sanitize(url) {
  // prepend default protocol if necessary
}

I know those upload URLs are hardcoded in file.ts, so we'd have to pick them up from the instance instead.

• Users would always handle the full URL path, as opposed to having to figure out which part we cut out
• It would allow both protocol and port to be overridden to match their mock env

Is this solution possible, or maybe not even a good option for what you think users would want?

it appears our benchmarking fake serves the API without the '/storage/v1' prefix.

We could probably change our benchmark mock? I still have the same concerns about what a user would find obvious when it comes to what path we send a request to. The easiest thing IMO is to only allow overriding the section in bold: protocol://host:port/path, regardless of when path starts with "storage/v1" or anything else.

@jkwlui Is this how it works currently? ^ the part about replacing the section in bold.

Thanks for bringing it up @stephenplusplus. I've decided to keep STORAGE_EMULATOR_HOST working as it is right now, since other languages also depend on the benchmark mock. I understand this is inflexible and potentially confusing, but since this should be a private option I think it's okay for now.