Inconsistent Configuration Fetching for SciCat Frontend
Issue Name
Inconsistent Configuration Fetching for SciCat Frontend
Summary
Currently, there is an inconsistency in how the SciCat frontend fetches its configuration. The frontend supports fetching configuration both locally (from a static file) and remotely (from the backend), leading to unnecessary complexity and potential confusion.
Steps to Reproduce
- Start the SciCat frontend application.
- Observe how the configuration is fetched: First, it attempts to fetch configuration from the backend at
/backend/api/v3/admin/config. If the backend configuration is unavailable, it falls back to fetching local configuration from/scicat/assets/config.json. - Analyze the code behavior and handling in case of errors or missing configurations.
Current Behaviour
The frontend uses a nested try-catch structure to manage configuration fetching, which is an anti-pattern for flow control.
This approach introduces confusion and tightly couples frontend behavior to backend configuration, violating separation of concerns.
try {
const config = await this.http
.get("/backend/api/v3/admin/config")
.pipe(timeout(2000))
.toPromise();
this.appConfig = Object.assign({}, this.appConfig, config);
} catch (err) {
console.log("No config available in backend, trying with local config.");
try {
const config = await this.http.get("/scicat/assets/config.json").toPromise();
this.appConfig = Object.assign({}, this.appConfig, config);
} catch (err) {
console.error("No config provided.");
}
}
Expected Behaviour
The backend should not be responsible for providing frontend configurations.
The frontend configuration should be purely local to the frontend to avoid unnecessary dependencies.
Error handling should avoid the use of try-catch for flow control and instead use a cleaner and more modular approach.
Extra Details
Refactor the configuration fetching logic to remove the backend dependency for frontend configuration.
Ensure error handling is clean and adheres to best practices.
If remote configuration is unavoidable, establish a clear distinction between backend and frontend configuration responsibilities.
This has been done in this way on purpose:
For a production deployment everything is served on one hostname, and the FE can fetch its config from the corresponding backend. You can then use one frontend-deployment to serve different backends on different hostnames.
The FE-local version is only suitable for dev setups without a proxy included, as there is no way to find where the backend should be. Maybe we should skip this option and just require dev setups with proxy.
Thank you for your clarification! I can see the intent behind the current design, but I believe rethinking deployment strategies can provide a more modular and maintainable solution.
For a production deployment everything is served on one hostname, and the FE can fetch its config from the corresponding backend. You can then use one frontend-deployment to serve different backends on different hostnames.
From this I take you actually need multiple customized FEs. Because this implies that different BE provide different FE config... This will be achieved much better with localized configs, especially if we talk K8s i.e. ConfigMaps. So there will be one BE deployment and multiple FE. This will also reduce coupling between two.
The FE-local version is only suitable for dev setups without a proxy included, as there is no way to find where the backend should be. Maybe we should skip this option and just require dev setups with proxy.
Not necessarily, I have dev setup with proxy and still there is no benefit in keeping FE config on the BE. In fact I completely got rid off it, by removing corresponding code lines.
A while back, the community discussed the issue and decided to serve the FE config from a BE endpoint.
We at ESS like this solution as we have only one place where to update configurations for SciCat. Also if in the future, we decide to provide an admin interface that allows configuration on the fly, we will need to move configuration to the database and, to avoid setting up a separate service, a BE dedicated endpoint for the FE configuration does the job.
That said, I see your point to decouple FE and BE and, I agree with you, that the code can improve.
I propose the following solution which should allow all use cases: create a run time configuration option that dictate how to load the configuration. Possible options:
- local. The configuration should be provided locally with app. No call to an external URL. The official FE release comes with the default one. You can customized it but it will require a rebuild of the app.
- remote. The configuration is fetched from an arbitrary URL provided in configuration. It can be the current BE URL or another one that is compatible and more suitable with the local infrastructure where SciCat is installed. If the instance admin decides to use the BE endpoint (which I would like to rename it), the functionalities stays as they are. Fall back will use the local FE config shipped with the FE.
Thoughts? Also would you like to work on such feature?
H-hm, an interesting case, indeed.
How will you force the clients to fetch the new configuration once it's changed?
What if the configuration scheme has to be updated with some incompatible changes?
Hi all,
One of the issues with the above-mentioned code snippet is that the part that tries to retrieve the configuration from the backend is using Angular's HttpClient like so:
const config = await this.http
.get("/backend/api/v3/admin/config")
.pipe(timeout(2000))
.toPromise();
But HttpClient is by default pointing to the frontend instead of the backend, which will always result in an error (notice port 4200 for fe instead of 3000 for be):
Failed to load resource: the server responded with a status of 404 (Not Found)
http://localhost:4200/backend/api/v3/admin/config
Are we trying to make a call to the backend without yet knowing the backend url, which comes from the config file that we are trying to read here? There seems to be some type of "circular dependency" that will always result to the configuration not being loaded from the backend but always being read from the local file.
I'm a bit late to the party here, but I just want to drop in and offer my endorsement for the "separation of concerns."
I think we can all agree that there is at least one piece of information that should always be stored with the front end, rather than fetched from the back end: The location of the back end.
But depending on how people want to customize the two, including potentially running multiple front ends, we could make a wide variety of cases for whether any configuration field affecting the front end should be stored with it. For example, the name of the institution one is logging into. Since the backend is handling authentication, it's reasonable to assume the backend would provide the name. But it's also clearly a UI concern, so there's an obvious case for storing it in the front end only.
Again, my apologies for being new here and not super familiar with the code as it stands, so perhaps the current implementation already works this way... But wouldn't it be a good idea to
- make a DTO defining all configuration fields that concern the front end, with all of them optional except the URL of the backend,
- express a version of that as JSON in the front end,
- then when the front end loads, immediately make an API call to the backend to fetch the same object,
- and overlay any defined fields returned from that object onto the one from the front end?
This allows people to store configuration in either place to suit their needs, and also allows for multiple front ends, and also provides a clear path to eventually move all configuration to the database level where it can potentially be edited with a UI.
I've had more time to look over the code, and I believe the solution I described above is an ideal one. In default cases of a single front end hosted on the same URL as the back end, an admin can leave the front end config.json entirely blank, and the front end can fall back to requesting config at the API point it does now. Then the back-end can provide config information however it wants.
The current implementation is broken for all scenarios except the default of:
- Front end config is stored on the back end
- Back end is at the same domain as the front end
Some detail:
Putting the front end at a different domain is broken because the front-end always tries to load config from /api/v3/config on its own domain, and if that fails, it pitches an error and halts, leaving the user with a blank screen. It never even attempts to load config.json from the front-end assets. Shouldn't it try that first?
Multiple front ends are broken because of the above reason, and because back-end post-login redirects are currently fixed to a location specified in the back-end config, and the 'use the referrer' and 'successUrl' methods for redirecting elsewhere are both broken. ( https://github.com/SciCatProject/scicat-backend-next/issues/1352 )
A path forward:
We could implement the logic I described above to get both these scenarios working and get better separation of concerns, but there is something to be wary of:
Right now there is an /assets/config.json file included in the standard SciCat front end image. Not /assets/config.example.json or something like that, but /assets/config.json, and it's full of default values. If we implement this logic and push it to a new SciCat version, everyone who pulls the new image will need to be aware that unless they're already customizing /usr/share/nginx/html/assets/config.json in their deployment, this new code will fetch that default config file and use anything in it that isn't explicitly defined in frontend.config.json in their back end. This will probably not be an issue for anyone, but, you never know...
@GBirkel thanks for the summary. At PSI we deploy the frontend and backend at different URLs, so we have to override /assets/config.json. I like your proposal.
So if I'm understanding correctly, we would fetch and merge the following three json objects:
/assets/config.defaults.json: defaults, eg the current/assets/config.json. This would havelbBaseURL: ""to support single-address deployments./assets/config.json: custom frontend config. In most cases this would only overridelbBaseURL.${lbBaseURL}/api/v3/admin/config: generated on the backend fromsrc/config/frontend.config.json
Currently the backend doesn't dynamically generate many values, but if that's expected to change in the future then implementing this 3-level configuration makes sense.
That looks like the right sequence, yes. I think there's one caveat when dealing with multiple front-ends:
We need to fetch and read /assets/config.json / /assets/config.defaults.json first in order to know where to look for the back end, to call ${lbBaseURL}/api/v3/admin/config. But we don't want the configuration returned by the back end to override lbBaseURL if it returns an empty default like "", or the front end suddenly won't be able to find the back end again.
We could just remove this parameter from the back end, so it's never merged in.
It's also never used by the back end. This may actually be an oversight, since we could be using it to assemble e.g. authentication redirects for OIDC instead of using an OIDC_CALLBACK_URL environment variable. (We know internally where our own OIDC callback should go, as long as we know the base URL of the back end, so it's actually cleaner to assemble it internally from lbBaseURL. But we don't currently do this.)
BUT, that would make this a parameter used by the back end as well as the front end, so it shouldn't be in src/config/frontend.config.json.
Food for thought. None of this prevents us from implementing this feature though.