Request resources asynchronously when the socket connection fails #1482
Conversation
| dispatch(receiveResource({ key, resource })); | ||
| }, | ||
| { | ||
| onErrorHandler: () => { |
There was a problem hiding this comment.
Should we only be doing this for a specific error?
There was a problem hiding this comment.
There are several reasons why the socket may fail: a network error, a forbidden error if the user doesn't have permissions to "watch" a resource, a timeout after sometime... In all of those cases it's fine to jump to the fallback IMO, that's why I am not filtering the error. Are you thinking on a specific case?
There was a problem hiding this comment.
Nope, I was just wondering if it was intentional that it was a catch all (ie. always drop to the fallback).
dashboard/src/actions/kube.tsx
Outdated
| dispatch(requestResource(key)); | ||
| // If it's not the first request, we can skip the request REDUX event | ||
| // to avoid the loading animation | ||
| if (shouldDispatchRequest !== false) { |
There was a problem hiding this comment.
Why not if (shouldDispatchRequest) { ? Ah, perhaps existing call-sites haven't been updated so get a default of undefined which isn't true? Not sure, but wonder if polling would be a better arg, which is fine defaulting to false/undefined, so here can be if(!polling) {...}. Anyway, fine either way, just found shouldDispatchRequest a little confusing at first because it implies it's talking about whether the actual request (to the server) will be dispatched, but it's not, it's just the redux state.
There was a problem hiding this comment.
Why not
if (shouldDispatchRequest) {? Ah, perhaps existing call-sites haven't been updated so get a default ofundefinedwhich isn't true?
yes, it's because of that
Not sure, but wonder if polling would be a better arg, which is fine defaulting to false/undefined, so here can be if(!polling) {...}. Anyway, fine either way, just found shouldDispatchRequest a little confusing at first because it implies it's talking about whether the actual request (to the server) will be dispatched, but it's not, it's just the redux state.
I think I don't understand what polling would mean in this context? I am okay changing the var name though.
There was a problem hiding this comment.
It wasn't so much about changing the name as opposed to choosing something such that the default value could be undefined/false, so that you could just do if (!polling) {...} (and perhaps be a little clearer). But totally fine as is, imo.
Description of the change
This PR creates an
onErrorhandler that can be called if the socket connection throws an error. This triggers an interval to re-request resources every 5s. This interval can be closed the same way than the socket callingcloseTimer.I have tested this removing the Nginx configuration that allows the socket connection, in the GIF below, you can see how the application gets updated even without the socket. When leaving the view, the timer gets cleared so we don't re-request more those resources.
Applicable issues