-
-
Notifications
You must be signed in to change notification settings - Fork 594
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
Improving Invalid Session Handling #307
Comments
It's definitely important for app developers to handle invalid sessions in cases where they might want to allow users to log out specific devices. However, I don't believe there is a general catch-all solution for this, since the cases that result in such an error are broad. The first solution you describe is a little complex, because applications can explicitly specify any token to make a request with (using the permissions options on any network request). Therefore, I could feasibly be logged in with one session for User A, and be using a different token for User A to make a request (or an unrelated token entirely). Just because that token fails doesn't mean that I should log out my current user, nor does it mean that I should specifically log out User A. You can see how such behavior would cause confusion for users being automatically logged out while making a simple query fetch (not to mention the breaking changes you mention). This solution is also hard to use in practice, because there is no universal log-out listener. Applications need to be able to redirect to the log-in screen whenever they are logged out, and if this behavior happens behind the scenes it's impossible to know this happened. The second suggestion is closer to what we suggest, but we don't believe there should be a universal global handler for the entire SDK. // BAD
doSomethingAync().then(() => {
// ...
}).then(() => {
// Any errors here aren't handled below
}, (err) => {
// ...
});
// GOOD
doSomethingAync().then(() => {
// ...
}).then(() => {
// Errors here are still handled!
}).catch((err) => {
// handle any and all errors
}); So I'd recommend to all that you use the second, but that it should not be a global property of the SDK. Having the freedom to implement this yourself also lets you chain together multiple handlers, should you choose to do so. function handleInvalidSession(err) {
if (err.code !== Parse.Error.INVALID_SESSION_TOKEN) {
return Parse.Promise.reject(err);
}
// ... deal with the invalid session
}
new Parse.Query('Item')
.find()
.then(processItems)
.catch(handleInvalidSession)
.catch(handleOtherErrors); In general, think of the SDK as moving towards a more explicit model with less "magic" or automatic behavior. These outcomes typically confuse developers, rather than making life easier. |
Thanks for the detailed response, @andrewimm. My instinct is to say that "simple things should be simple, and complex things should be possible". It seems like the common use case is that you are making a request on behalf of an authenticated user, and that the SDK should understand when that state changes and provide a clean way for a developer to hook in to that event. While the explicit behavior you describe nicely follows the principle of least surprise, it also leaves a lot of that responsibility on the developer. I would further argue that when a developer is opting in to more complex behaviors, such as making a request with an explicit session token, it's also acceptable for them to take on the burden of managing this kind of stateful session behavior instead of letting the SDK handle it for them. Having said that, I understand the argument that "explicit is better than implicit", and a desire as a maintainer to avoid features that have been seen to confuse the developers using the SDK. Seeing as this is documented behavior with a clear rationale, I'm closing this issue. Thanks! |
@noahsilas I do appreciate you taking the time to voice your concerns, and suggest improvements. I see this SDK as an evolving codebase, and it's crucial for the many users to continue contributing ideas to make it better each day. |
Hey, I have built a query builder. |
According to the documentation on Handling Invalid Session Token Error, the appropriate resolution is to attach an error handler to every call out to the Parse server.
Applying this change to an existing aplication is a bit of a chore in terms of finding all of the appropriate places to add the error handler. It also places a burden on developers to remember to include this handler whenever writing new code.
I can think of a couple ways to address this:
Always log the user out on INVALID_SESSION_TOKEN
We build in to the framework a special case for this error that directly invokes
Parse.User.logOut
. This has the benefit of immediately changing the state ofParse.User.current
to reflect the new information, which may be sufficient for some applications to understand the state change. The downside is that this may be a breaking change in the SDK for some applications that aren't expecting it.Allow registration of a global error handling callback
Imagine initializing the SDK could look something like:
When this handler is present, it could be automatically invoked whenever we receive an error response from the Parse server. This has the downside of being extra configuration, but is extremely flexible in how it could be used. It also doesn't present any possibility of being an API breaking change, as the behavior would remain the same if the global handler is not registered.
How would folks feel about a pull request for either of these strategies? Are there other options to improve this that folks would rather see?
The text was updated successfully, but these errors were encountered: