-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
feat: add renew token query for apollo client (chrome-extension) #5200
feat: add renew token query for apollo client (chrome-extension) #5200
Conversation
const getAuthToken = async () => { | ||
const store = await chrome.storage.local.get(); | ||
if (isDefined(store.accessToken)) return `Bearer ${store.accessToken.token}`; | ||
else return ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we return null instead to signify that we couldn't get it ? Seems that '' will create a silent error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to return null 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like HttpLink shouldn't be created if we don't have any accessToken, but instead redirect to the login page no ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's the plan! We currently don't have any routing in place though its next on the agenda :)
({ graphQLErrors, networkError, forward, operation }) => { | ||
if (isDefined(graphQLErrors)) { | ||
for (const graphQLError of graphQLErrors) { | ||
if (graphQLError.message === 'Unauthorized') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There might be a problem here, because the server doesn't necessarily returns this exact error.
I'm looking into it, might be a problem of guard.
@AdityaPimpalkar Could you look into that issue with @Weiko #5224 it's currently causing problem to validate this PR properly. Blocked by #5224 |
Hi @AdityaPimpalkar, we fixed the problem, the right errors are now thrown by the backend. I tested and I now get an error when I try to login with the extension : |
Hey @lucasbordeau Thanks! |
fixes - #5203