session icon indicating copy to clipboard operation
session copied to clipboard

Handle store errors directly.

Open Fishrock123 opened this issue 11 years ago • 5 comments

We should probably directly handle store errors, rather than the abstracted disconnect.

Fishrock123 avatar Aug 19 '14 15:08 Fishrock123

I'm subscribing to this issue and am available to discuss any solutions here, as it makes sense to handle the errors in one place with one common workflow for store implementors.

wavded avatar Aug 29 '14 16:08 wavded

So in most causes, I'm' not sure we need to do anything special for a "disconnected" store--especially since most stores would try to reconnect automatically anyway. All we should be doing it calling store.get like we do now, but unconditionally. Then the store can choose to either queue things up while it is reconnecting or to send back errs.

dougwilson avatar Aug 29 '14 16:08 dougwilson

I typically suggest just doing this:

app.use(session)
app.use(handleSessionError) // does req.session exist?

But it seems people want to be notified otherwise. Like in the case where the server starts and store.get fails immediately. I suppose store.get could always be called regardless of its disconnect status, just trying to think of how/if that would screw up existing stores.

wavded avatar Aug 29 '14 17:08 wavded

Like in the case where the server starts and store.get fails immediately.

Well, what I''m saying is that while like Redis is connecting, store.get can just queue up all the requests (handled by the store) and then do all the accumulated lookups as soon as it connects.

just trying to think of how/if that would screw up existing stores.

This would be a 2.0 change. We may add something that stores would need to implement so we know if it's a 1.x-style store or a 2.x-style store and act accordingly.

dougwilson avatar Aug 29 '14 17:08 dougwilson

Is there anything actionable here?

gireeshpunathil avatar Oct 30 '19 06:10 gireeshpunathil