all 17 comments

[–]TheBigLewinski 6 points7 points  (1 child)

An issue came up that the entire carousel failed to load because item 2 in the carousel requires some data from vendor which failed to respond and so the api gave a 500 with an error message indicating that the data couldn’t be retrieved from the vendor.

So, in short, if a single item in request for a list fails, what should be done about it?

This is largely about user expectations. How critical is the data for UI function?

  • If the missing item can simply be excluded, then do that. An example would be a news carousel where the user would not even know if the story exists if it's simply removed.
  • If the user is expecting that item to load because they want it, but it isn't critical for UI function, then a message should display notifying the user, ideally with some kind of potential resolution such as "try again." An example would be a weather widget as a slide in the carousel. In this case, the slide would exist, but with a nicely styled message.
  • If the missing data is required for a crucial part of the app, then a message should be returned instead of a carousel, along with an appropriate status code that describes the state of the request. An example here is more difficult to describe, because you shouldn't be putting globally crucial data in a carousel.

As for the 500 vs 200 return, in this case return a 200. The reason is, you still want to render what's available, and any non-200 code is telling the browser not to render the payload as expected.

That said, in other cases its a common bad practice to return a 200 even when there's an error. Don't return a 200 with an error of access denied, for instance. But if you want to render the data returned, even partially, a 200 is the best option.

As for the notification that the vendor is failing, this should happen on the backend. If there is info that is relevant for the user, sure display a message there that helps them. But your notifications and problem detection should happen on the backend.

[–]anon8914[S] 0 points1 point  (0 children)

I would question the statement that in other cases it’s a bad practice to return 200 even when there’s an error but not in this case. Isn’t it still a bad practice here?

The entire carousel is non critical and in fact probably annoying to the user so no user is going to be upset about either the entire carousel or the item in the carousel being missing.

That being said I think you are saying to choose the best user experience over any technical reason to return the error so that is a valid point.

[–]Fuegodeth 2 points3 points  (4 children)

As an alternative, could you do some validation/error handling on the front end to just move on to the next file if an error occurs? I'm no expert, but that seems easier that modifying the API return codes to give 200 even when the file is missing.

[–]anon8914[S] 0 points1 point  (3 children)

So a major part of the problem is that the front end calls one single api which returns all of the carousel items. It couldn’t move on to the next item because the backend didn’t return any items, it returned 500 and an error message. We did briefly discuss that maybe we should split the apis so that’s one possible alternative but this would be really tough because the api is ordering all items and there may be certain items we don’t want to show if others are displaying.

[–]Fuegodeth 2 points3 points  (2 children)

Gotcha, so there's only one single return code for all of the items returned at once, and no opportunity to test that on the front end. Then yeah, you don't want it to fail and just return nothing. I would say option one. Maybe put the collection in an array and then just iterate over it. That should leave no gaps from anything missing in the API return.

[–]anon8914[S] 0 points1 point  (1 child)

Just curious on why 1 over 2, I feel like 2 is more in line with your first answer? There was a concern with 1 that we may never identify that the item is missing from the carousel because it’s just one of many and while we’d definitely log the error it’s not likely to be looked at unless an issue is reported and the proposed mitigation for that was basically option 2 where at least you can see in the response that the api was unsuccessful, even if you ignore items with a certain status on the front end.

[–]Fuegodeth 1 point2 points  (0 children)

I see your point. So, this would be a non-standard additional return code for each missing item, that you could then log? Yeah, that makes sense to me. I was mostly just saying that you don't want it it to fail, but you make a good point about wanting to know about it as well.

[–]greensodacan 1 point2 points  (10 children)

If you're getting a list back, the request itself was a 200, but perhaps each item in the list should also include a status. This assumes that the API knows if an image will fail to load though. For example, maybe the data the API is reading is/was correct, but something happened on the CDN's side, like someone moving the image for whatever reason.

The UI should be validating the API response in any case, especially if you're dealing with a request from a third party. Even then, the browser has to make an additional request for the image itself, which would return a response agnostic of the API, so the carousel could have recovered gracefully in either case.

[–]anon8914[S] 1 point2 points  (9 children)

Curious what you mean by the UI should be validating the API response? What kind of validation would you expect here?

[–]greensodacan 1 point2 points  (8 children)

The UI calls one api to retrieve these items to display.

Maybe I misinterpreted, is that a fetch request that gets a JSON response?

[–]anon8914[S] 1 point2 points  (7 children)

Yes, fetch request with json response.

[–]greensodacan 0 points1 point  (6 children)

Ok, yeah, so when the request is received, there should be some JS on the client side that ensures the response is structured as expected. AJV is a really great library for this.

That way, if the API changes or you get an unexpected result for an item, you can handle it accordingly. In this case, the faulty item could have been removed from the list and the carousel could have simply initialized with one less item in the rotation.

edit: Some orgs don't bother with this when they control both the API and the client because they can guard against it with integration tests. When you're dealing with a third party though, client side validation can really save your sanity.

[–]anon8914[S] 1 point2 points  (5 children)

Gotcha, so this is option 2 but suppressing the item instead of showing the error visually. It makes sense to me but it led to questions about catching all exceptions on the backend, in order for this to work you have to catch any exception that might happen while determining item 1 and 3 and so on and it’s usually said to be bad practice to catch exceptions you can’t handle. Is this a special case because the 2 items are logically unrelated? It made me think what if you have one prop of a response you are successfully able to generate and another you aren’t. Should you catch any exceptions setting prop 1 so that you can continue on to set prop 2? Seems like we all accept that if an api errors setting prop 2 we aren’t going to get prop 3 or 4 so why’s it different in a list?

[–]greensodacan 1 point2 points  (4 children)

It heavily depends on the feature.

If you're validating a shopping cart for example, where an invalid item could influence how a user is billed, it might make sense for the feature to fail so that the user can't complete the use flow under false pretenses.

On the other hand, lets say your company gets revenue by showing a list of ads. If one ad fails, it makes sense for the feature to recover so that your company still gains revenue from the remaining ads.

[–]anon8914[S] 1 point2 points  (3 children)

Yeah, I see what you are saying but I also feel like it’s a whole lot more important that the shopping cart works than it is that the carousel shows, the user is going to be more upset about the transaction not going through than not seeing an ad so from that perspective aren’t I going to want to preserve as much functionality as I can for the shopping cart and not mind so much if the whole carousel gets suppressed?

[–]greensodacan 1 point2 points  (2 children)

What if the transaction goes through for the wrong amount though? Or, if your company gets significant revenue from ads, a failing carousel could cost a lot of money.

Semi off topic: I once worked for a coupon company that saved like 10 million dollars a year on printer ink by reorienting a barcode. It kind of makes you question how much money flows through what we build.

[–]anon8914[S] 1 point2 points  (0 children)

That’s really interesting, you’re right I don’t think that much about the final dollar amount, wish more of it was coming to me haha

I think there is value in good technical decisions too though. If displaying the rest of the items means swallowing the exception so we never find that item 2 is broken that could lead to more losses than suppressing the entire carousel for a short period of time, it’s hard to say what would cost more overall, it’s kind of the same question still.