Skip to content
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

Read frontend API_URL environment variable at runtime #3934

Closed
sarayourfriend opened this issue Mar 19, 2024 · 2 comments
Closed

Read frontend API_URL environment variable at runtime #3934

sarayourfriend opened this issue Mar 19, 2024 · 2 comments
Labels
🕹 aspect: interface Concerns end-users' experience with the software 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend

Comments

@sarayourfriend
Copy link
Collaborator

Problem

Because of the way Nuxt's build works, process.env.API_URL in the env.ts file is "baked" at build time, so that it statically references the value at build time:

const apiUrl = process.env.API_URL ?? "https://api.openverse.engineering/"

Despite this seeming not to be the case, when you investigate the built code:

// .nuxt/dist/server/server.js
"use strict";
/* harmony export (binding) */ __webpack_require__.d(__webpack_exports__, "a", function() { return env; });
var _a;
const apiUrl = (_a = process.env.API_URL) !== null && _a !== void 0 ? _a : "https://api.openverse.engineering/";

It is actually baked into the Nuxt context:

// .nuxt/dist/server/server.js
async function setContext(app, context) {
  // If context not defined, create it
  if (!app.context) {
    app.context = {
      isStatic: false,
      isDev: false,
      isHMR: false,
      app,
      payload: context.payload,
      error: context.error,
      base: app.router.options.base,
      env: {
        "apiUrl": "https://api.openverse.engineering/",
        "filterStorageKey": "openverse-filter-visibility",
        "savedSearchCount": "4",
        "providerUpdateFrequency": "3600000"
      }
    };

// ...

const createApiService = ({
  baseUrl = "https://api.openverse.engineering/",
  accessToken = undefined,
  isVersioned = true
} = {}) => {

This is complicated by the fact that we do not use Nuxt context to retrieve the default value for baseUrl, meaning we necessarily rely on static definitions, that Nuxt will overwrite.

This documented and intentional Nuxt behaviour: https://v2.nuxt.com/docs/configuration-glossary/configuration-env/#processenv--

Description

To fix this, we could switch to using publicRuntimeConfig, which will read the environment variable at runtime. However, that makes things a big more complicated, because we'd be forced to rely on the Nuxt context $config to retrieve the variable! That's very hard with the way we've written the api-service.ts module and created top-level calls to createApiService that cannot reference runtime context.

While that would be the "correct" solution, we can bypass the static injection of the variable at build time by using process.env.API_URL instead of the nuxt env setting's apiUrl. That would require a one-line change, and define the default where it's used, rather than in the global "env" context.

The following patch would do that:

diff --git a/frontend/src/data/api-service.ts b/frontend/src/data/api-service.ts
index f67bc1024..6797f236e 100644
--- a/frontend/src/data/api-service.ts
+++ b/frontend/src/data/api-service.ts
@@ -92,8 +92,15 @@ export interface ApiService {
   ): Promise<AxiosResponse<T>>
 }
 
+function getDefaultApiUrl() {
+  const defaultApiUrl = process.env.API_URL || "https://api.openverse.engineering/"
+  return defaultApiUrl.endsWith("/") ? defaultApiUrl : `${defaultApiUrl}/`
+}
+
+const DEFAULT_API_URL = getDefaultApiUrl()
+
 export const createApiService = ({
-  baseUrl = process.env.apiUrl,
+  baseUrl = DEFAULT_API_URL,
   accessToken = undefined,
   isVersioned = true,
 }: ApiServiceConfig = {}): ApiService => {
diff --git a/frontend/src/utils/env.ts b/frontend/src/utils/env.ts
index 8d089ce14..ba439f330 100644
--- a/frontend/src/utils/env.ts
+++ b/frontend/src/utils/env.ts
@@ -1,4 +1,3 @@
-const apiUrl = process.env.API_URL ?? "https://api.openverse.engineering/"
 
 /**
  * Default environment variables are set on this key. Defaults are fallbacks to existing env vars.
@@ -6,7 +5,6 @@ const apiUrl = process.env.API_URL ?? "https://api.openverse.engineering/"
  * providerUpdateFrequency - how often we should re-fetch provider statistics.
  */
 export const env = {
-  apiUrl: apiUrl.endsWith("/") ? apiUrl : `${apiUrl}/`,
   filterStorageKey: "openverse-filter-visibility",
   savedSearchCount: "4",
   providerUpdateFrequency: `${60 * 60 * 1000}`, // 1 hour

It (a) removes the env.apiUrl definition, which has the "baked in" feature, as described in the above section, and (b) changes the createApiService function to use a new default that reads from the environment at runtime, falling back to the currently established default.

This gets around the build-time injection of the value in the code, and makes it possible to point the built frontend to any API URL desired, just by using the API_URL environment variable.

Alternatives

I mentioned the publicRuntimeConfig alternative. It is better in that it is more "nuxt-y" and would force us into a more maintainable pattern for our API service instance creation.

However, I think the best and most flexible option, would be the following:

  1. Create a Nuxt plugin that defines the API service instances (or, better yet, create a single API client instance based on something like the JS client https://git.sr.ht/~sara/openverse-api-client generates or https://git.sr.ht/~sara/openverse-api-client-js defines)
  2. In that plugin, at request time, resolve the API URL to use based on the following logic, in order:
    • If the requester has manually set a session variable apiUrl, use that.
    • Otherwise, if process.env.API_URL is defined, use that.
    • Otherwise, use https://api.openverse.engineering/ as the absolute default.

This is the best solution because it allows us to change the API url at any time we want for any environment, in order to test whatever API url we like. If we want to point the production frontend to the staging API, we can do so for ourselves without affecting anyone else. Likewise, if we want to default the staging frontend to point to the staging API, we can do that, while allowing developers to easily point the staging frontend to the production API by setting the session variable. Or if we don't want that default, we can individually point the staging frontend to the staging API whenver we want.

It's tremendously flexible, but would require a significant overhaul in our API request handling on the frontend. It's something we should do, but difficult to argue needs to be done, considering the effort it would take, and the fact that there is a usable workaround the achieves the main, immediate needs.

@sarayourfriend sarayourfriend added 🟨 priority: medium Not blocking but should be addressed soon 🕹 aspect: interface Concerns end-users' experience with the software 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🧱 stack: frontend Related to the Nuxt frontend labels Mar 19, 2024
@openverse-bot openverse-bot moved this to 📋 Backlog in Openverse Backlog Mar 19, 2024
@obulat
Copy link
Contributor

obulat commented Mar 19, 2024

I'd prefer the second solution because we have discussed moving away from the api service anyway.

Using the nuxt context is problematic because it's not always available when we need it. Namely, on the server, when we do async fetch or call a function that makes an api request inside the async fetch or in the middleware. At least, that was the problem I had in the Nuxt 3 branch.

@sarayourfriend sarayourfriend changed the title Move frontend env variables into publicRuntimeConfig Read frontend API_URL environment variable at runtime Mar 19, 2024
@obulat
Copy link
Contributor

obulat commented Aug 23, 2024

This has been fixed in #4705

@obulat obulat closed this as completed Aug 23, 2024
@openverse-bot openverse-bot moved this from 📋 Backlog to ✅ Done in Openverse Backlog Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🕹 aspect: interface Concerns end-users' experience with the software 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend
Projects
Archived in project
Development

No branches or pull requests

2 participants