Comments (6)
Thanks, @clement911.
- For me, an exception that doesn't come from Shopify API should be treated as a library exception
ShopifySharpException
instead to avoid confusion. - I also don't think
ShopifyGraphException
should inherit fromShopifyHttpException
. They both should inherit fromShopifyException
In summary, the hierarchy I see is:
- ShopifySharpException (treat library exceptions with the inner exception of what happened)
- ShopifyException (To group all Shopify API exceptions regardless of type)
- ShopifyGraphException (Specific to Graph error results)
- ShopifyRestException (Specific to REST errors / Http Status codes)
Anyways, you guys have done an excellent job with the library, I just wanted to add my 2 cents.
from shopifysharp.
I've been caught by this as well.
However, if this new exception type doesn't inherit from ShopifyHttpException
, this might be an issue as well.
One might catch ShopifyHttpException
(as I do) and expect to get all shopify API errors, but would not.
I wanted to catch Shopify internal error. I solved it with a simple extension.
public static bool IsShopifyInternalError(this ShopifyHttpException shEx)
{
return shEx.HttpStatusCode == HttpStatusCode.InternalServerError ||
//graphql error returns HTTP 200 with the error desc in the response
(shEx.HttpStatusCode == HttpStatusCode.OK && shEx.RawBody?.Contains("INTERNAL_SERVER_ERROR") == true);
}
I'm curious under what scenario you need to filter for specific HttpStatus
?
from shopifysharp.
My point of view:
They should all extend ShopifyException
. GraphQL errors have a different schema so we should be able to get that information if we get one.
GraphQl Error example:
{
"errors": [
{
"message": "Query cost is 2003, which exceeds the single query max cost limit (1000).See https://shopify.dev/concepts/about-apis/rate-limits for more information on how thecost of a query is calculated.To query larger amounts of data with fewer limits, bulk operations should be used instead.See https://shopify.dev/tutorials/perform-bulk-operations-with-admin-api for usage details.",
"extensions": {
"code": "MAX_COST_EXCEEDED",
"cost": 2003,
"maxCost": 1000,
"documentation": "https://shopify.dev/api/usage/rate-limits"
}
}
]
}
The proper way to handle would be to catch ShopifyException
exceptions and then filter what type you want using is
if you are interested in additional details.
try
{
...
}
catch (ShopifyException exception)
{
if (exception is ShopifyHttpException httpException)
{
//use httpException variable which will be casted to ShopifyHttpException
}
else if (exception is ShopifyGraphResultException graphqlException)
{
//use graphqlException variable which will be casted to ShopifyGraphResultException
}
}
Not sure how you both feel about this moving forward. This can be treated as a breaking change if people are just using ShopifyHttpException
for all.
from shopifysharp.
Maybe the following hierarchy would be appropriate?
- ShopifyException
- ShopifyApiException
- ShopifyAdminApiException
- ShopifyAdminApiRestException
- ShopifyAdminApiGraphException
- ShopifyPartnerApiException
- ShopifyStorefrontApiException
- ShopifyAdminApiException
- ShopifyApiException
from shopifysharp.
Hi @clement911, I have two questions:
- Why would we need
ShopifyApiException
? There won't be other types ofShopifyException
that are not Api Exceptions, so we can reduce the hierarchy. - What is the value of separating by AdminApiGraph, Partner and StoreFront if they are all graphql APIs?
I would try to keep the hierarchy simple, but that's just my opinion. I want to understand better your point of view in case I'm missing something.
from shopifysharp.
- Yes there can be non-api exceptions such as serialization exceptions, null reference exception and any kind of exceptions related to other bugs. While ShopifySharp is a fairly thin wrapper around the APIs, there is still some code which can have bugs and exceptions.
- I'm not sure if there is any value. I'm not sure what the best design but that I was just offering a potential design. Maybe there should be just be a
ShopifyRestException
and aShopifyGraphException
. Graph errors usually still return an HTTP 200 OK, so it feels a bit weird to have those inherit fromShopifyHttpException
.
from shopifysharp.
Related Issues (20)
- Add product into Exising Channels HOT 4
- Unable to add multiple images to product using ShopifySharp HOT 7
- Product Service ListAsync - Not Paging HOT 2
- Deprecated property delivery_category is being used HOT 2
- How to list order by product ? HOT 3
- API get checkout by token not working HOT 6
- Internal application stopped working HOT 2
- Order not coming in api HOT 2
- Adding Multiple Tracking Numbers with Fulfillment.TrackingNumbers HOT 3
- Add read_returns and write_returns to access scopes enums HOT 2
- Customer is not returned by graphql mutation HOT 2
- LeakyBucket does not release the Semaphore if a request fails or the task is canceled
- Missing Alt attribute in ArticleImage class
- HttpResponseMessage Response in RequestResult marked as Obsolete HOT 5
- Add support for IAsyncEnumerable on every service that supports ListAsync
- Add support for validating the timestamp with a default timeout period in `ShopifyRequestValidationUtility`
- IsValidShopDomainAsync should check if shopDomain is a valid uri before building the shop domain uri HOT 2
- Support for Meta Objects? HOT 6
- RefundLineItem SubTotalTaxSet - copypaste issue? HOT 1
- Feature: Generate methods based on .graphql files HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from shopifysharp.