fix(url): Removed possible extra character in base URLs, to avoid issues when building path for requests.#516
fix(url): Removed possible extra character in base URLs, to avoid issues when building path for requests.#516
Conversation
…ues when building path for requests.
src/api-client/base-client.ts
Outdated
| headers = { ...headers, ...extraHeaders }; | ||
|
|
||
| const endpointPath = `${this.apiUrl}/${endpoint | ||
| const endpointPath = `${this.apiUrl.replace(/\/$/, '')}/${endpoint |
There was a problem hiding this comment.
How about doing this in BaseClient instead:
private _apiUrl: string = '';
protected get apiUrl(): string {
return this._apiUrl;
}
protected set apiUrl(url: string) {
this._apiUrl = url.replace(/\/$/, '');
}
// ... rest unchanged
There was a problem hiding this comment.
Commit f112c61 addressed this comment by introducing a private _apiUrl field with getter/setter logic that strips trailing slashes when the URL is assigned, allowing the request-building code to rely on this.apiUrl without additional trimming.
There was a problem hiding this comment.
Refactored for getter-setter.
src/api-client/base-client.ts
Outdated
| headers = { ...headers, ...extraHeaders }; | ||
|
|
||
| const endpointPath = `${this.apiUrl}/${endpoint | ||
| const endpointPath = `${this.apiUrl.replace(/\/$/, '')}/${endpoint |
There was a problem hiding this comment.
src/legacy-api-client.ts line 186 has the same pattern. We should probably do the same there.
Or, if you go with the setter approach in BaseClient, this would be automatically covered
since LegacyApiClient extends BaseClient and its this.apiUrl = this.getApiUrl() assignment would go through the setter.
There was a problem hiding this comment.
Commit f112c61 addressed this comment by adding an apiUrl setter on BaseClient that strips trailing slashes before storing the value and by relying on that sanitized property when building endpoints, so any subclass (including LegacyApiClient) now inherits the normalized URL without needing its own replace(//$/, '') logic.
There was a problem hiding this comment.
Refactored for getter-setter.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #516 +/- ##
==========================================
+ Coverage 75.21% 75.22% +0.01%
==========================================
Files 75 75
Lines 6741 6737 -4
Branches 1860 1854 -6
==========================================
- Hits 5070 5068 -2
+ Misses 1660 1658 -2
Partials 11 11
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
2 participants
User description
Possible E2E test issues with newly introduced features - sc-57125
Description
While fixing issues with e2e tests, it was observed the way base URLs were used could have issues if the final slash character was already included. Fixed accordingly.
Important: Have the following PR approved before, this one should then update galileo-generated's version.
rungalileo/galileo-ts-generated#57
Generated description
Below is a concise technical summary of the changes proposed in this PR:
graph LR BaseClient_("BaseClient"):::modified GALILEO_API_("GALILEO_API"):::modified log_("log"):::modified GALILEO_LOGGER_("GALILEO_LOGGER"):::modified BaseClient_ -- "Adds apiUrl property with trailing-slash trimming." --> GALILEO_API_ log_ -- "Prefers experimentContext metadata for logger configuration." --> GALILEO_LOGGER_ classDef added stroke:#15AA7A classDef removed stroke:#CD5270 classDef modified stroke:#EDAC4C linkStyle default stroke:#CBD5E1,font-size:13pxNormalize how
BaseClientstores the API base URL so it strips any trailing slash before building request paths, and keep the getter/setter private to enforce sanitized values. Remove theloggerContextfallback inlogso only experiment context data determines logger selection, preventing stale log store data from overriding experiment information.BaseClientby trimming any trailing slash when settingapiUrl, ensuring downstream request paths are built from a consistent root.Modified files (1)
Latest Contributors(2)
log, removing the fallback tologgerContextso stale log store values no longer override the active experiment details.Modified files (1)
Latest Contributors(2)