-
Notifications
You must be signed in to change notification settings - Fork 3
feat:error handling and region support #138
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances error handling and adds support for hosting service URLs. Key changes include:
- Introducing new type definitions (ServiceURLsMap, RequestInitConfig, RequestConfigWithBaseUrl) to support service URLs.
- Updating API request adapters and utility functions to integrate error handling and URL resolution.
- Modifying the UI location module to utilize the new hosted endpoints configuration.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/types/api.type.ts | Added new types for service URL and request configuration support. |
src/utils/adapter.ts | Updated dispatch functions with improved error handling and integration of hosted URLs. |
src/utils/utils.ts | Modified fetchToAxiosConfig and added isAbsoluteURL with baseURL resolution. |
src/uiLocation.ts | Updated the API method to include hostedEndpoints for service URL support. |
src/types.ts | Integrated new hosted endpoints config and extended region enums. |
Comments suppressed due to low confidence (1)
src/utils/utils.ts:54
- Consider adding an explicit type annotation to the 'url' parameter (e.g., url: string) to improve type clarity and safety.
function isAbsoluteURL(url) {
src/utils/adapter.ts
Outdated
return response | ||
|
||
} catch (error) { | ||
throw handleApiError(error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to throw this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need errors to be catched on user side right? If I do return errors are going as response
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But you are returning response object in all cases right? Can check your implementation is fetch compatible?
Signed-off-by: Amitkanswal <[email protected]>
d6daa79
to
9976df2
Compare
Signed-off-by: Amitkanswal <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces improved error handling and support for hosting service URLs by updating error processing logic and configuration types. Key changes include:
- Adding a new function (handleApiError) for unified error responses.
- Refactoring region handling to use a broader type (RegionType) and incorporating endpoints into configuration.
- Adjusting adapter logic to include additional response properties and updating tests accordingly.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/utils/utils.ts | Introduces new error handling utilities and region logic |
src/utils/adapter.ts | Refactors API dispatch, adding unified error handling |
src/uiLocation.ts | Adjusts region type usage and adds endpoints support |
src/types/api.type.ts | Adds endpoints type |
src/types.ts | Updates region and endpoints integration |
test/*.test.ts | Updates tests to align with new region and endpoints changes |
closing this PR as it was pending for long time creating a new PR |
Support-