STORAGE_EMULATOR_HOST not handled correctly

[mgabeler-lee-6rs]

This is a rehash of #1314, which was somehow "fixed" by adding comments internal to the implementation.

Environment details

• OS: Linux
• Node.js version: 14.x
• npm version: doesn't matter
• @google-cloud/storage version: 5.8.5

Steps to reproduce

1. Set STORAGE_EMULATOR_HOST to http://localhost:NNN
2. Try to use the library

As described in #1314, the distinction in the Storage constructor between apiEndpoint and baseUrl is not handled correctly when this environment variable is used. There is no possible value for this environment variable that results in a fully functioning client.

The proposed fix in that issue also seems to work just fine, so will submit a PR

[shaffeeullah]

Hi @mgabeler-lee-6rs ,

Thanks for following up on this issue. Is there a reason that apiEndpoint doesn't satisfy your use case? EMULATOR_HOST is an experimental variable meant to be used for some internal tests.

[mgabeler-lee-6rs]

Setting apiEndpoint requires modifying every application, often in multiple places, creating frustration & confusion, especially if some applications are third party.

Having an environment variable like this is incredibly handy for developer environments, for the same reason as the PUBSUB_EMULATOR_HOST is similarly helpful for that system. Multiple emulators for GCS are available (my team is looking at https://github.com/fsouza/fake-gcs-server FWIW), and being able to use one would greatly simplify my organization's developer environments.

From a more abstract point of view, in the current implementation there is no possible value for the STORAGE_EMULATOR_HOST environment variable that works properly, and that seems like it ought to be fixed :)

[mgabeler-lee-6rs]

Also, that environment variable is documented as "publicly" supported by the Go client, and I would hope that parity is desirable: https://pkg.go.dev/cloud.google.com/go/storage#hdr-Creating_a_Client

Since my org uses a mix of NodeJS and Go, such parity would certainly benefit us :)

[shaffeeullah]

@mgabeler-lee-6rs ,

Thanks for explaining this! I can totally see how the current developer experience is frustrating for your situation. I'm discussing this issue with our internal partners (like Go) to make sure that our implementations are consistent. I'll keep this issue updated.

[mgabeler-lee-6rs]

@shaffeeullah any updates you can share from those discussions?

[shaffeeullah]

@mgabeler-lee-6rs This looks good! 🎉 There is a code freeze in place until October 11. We will release this change after the freeze ends.

[mgabeler-lee-6rs]

That sounds great, thank you!

[mgabeler-lee-6rs]

Since the proposed fix broke something and was insta-reverted, can this be re-opened pending finding a more compatible solution?

[danielbankhead]

@mgabeler-lee-6rs I created a follow-up issue: #2092

This will be a breaking change and we'll need to coordinate a release with Firebase.