Skip to content
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

Recover lost connection #8284

Merged
merged 3 commits into from May 12, 2023
Merged

Conversation

standuprey
Copy link
Contributor

This is in reference to #8051

This PR makes sure that even if dbAuth's getToken fails (if for example the user loses his internet connection momentarily) the next call to the API once online again doesn't fail

@thedavidprice thedavidprice requested a review from jtoar May 11, 2023 02:28
@jtoar jtoar added the release:fix This PR is a fix label May 12, 2023
Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch @standup75 thanks for going through a bout of debugging here on our part and your reproductions/gifs were super helpful; let me turn this around in a patch for you

@jtoar jtoar merged commit b36bb13 into redwoodjs:main May 12, 2023
9 of 10 checks passed
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone May 12, 2023
jtoar pushed a commit that referenced this pull request May 12, 2023
@jtoar jtoar modified the milestones: next-release-patch, v5.1.4 May 12, 2023
jtoar pushed a commit that referenced this pull request May 12, 2023
@jtoar
Copy link
Contributor

jtoar commented May 12, 2023

@standup75 this is now available in https://github.com/redwoodjs/redwood/releases/tag/v5.1.4

dac09 added a commit to dac09/redwood that referenced this pull request May 12, 2023
…te-default

* 'main' of github.com:redwoodjs/redwood: (23 commits)
  fix: remove react 17/18 warning (redwoodjs#8300)
  chore(release): tolerate lerna publish faliure
  Recover lost connection (redwoodjs#8284)
  chore(deps): update dependency @faker-js/faker to v8 (redwoodjs#8296)
  chore(release): better git commits during release
  feat: experimental - Studio Overview and Performance Widgets (redwoodjs#8292)
  fix(forms): disable webpack-dev-server overlay (redwoodjs#8298)
  Fix studio lint warning (redwoodjs#8297)
  Fastify server: Default to localhost (redwoodjs#8019)
  Fix GraphQL proxy in dev environments without IPv6 (redwoodjs#8233)
  fix(deps): update dependency @graphiql/plugin-explorer to v0.1.18 (redwoodjs#8290)
  chore(deps): update dependency supertokens-auth-react to v0.32.3 (redwoodjs#8289)
  Add `setup sentry` command (redwoodjs#7790)
  chore: readme update core team and all contributors (redwoodjs#8288)
  fix(deps): update nivo monorepo to ^0.83.0 (redwoodjs#8286)
  fix(deps): update dependency babel-plugin-polyfill-corejs3 to v0.8.1 (redwoodjs#8281)
  chore(deps): update dependency @replayio/playwright to v0.3.30 (redwoodjs#8282)
  fix(deps): update dependency webpack to v5.82.1 (redwoodjs#8283)
  Add epilogue to builders (redwoodjs#8285)
  feat(studio): v2 studio (redwoodjs#8173)
  ...
Tobbe added a commit that referenced this pull request Sep 5, 2023
We have this comment in `useToken()`


https://github.com/redwoodjs/redwood/blob/4b734d30c6830172194a2518ae0c2cbf6f1a0904/packages/auth/src/AuthProvider/useToken.ts#L7-L14

And for example our Netlify auth implementation does this:


https://github.com/redwoodjs/redwood/blob/4b734d30c6830172194a2518ae0c2cbf6f1a0904/packages/auth-providers/netlify/web/src/netlify.ts#L68-L78

But dbAuth let the exception slip through to `useToken`.
This PR adds a `catch` to the promise and returns `null`, just like
Netlify (and others)


Before merging this though, I need to make sure I'm not undoing the fix
@standup75 did in #8284
(@standup75 can you say off the top of your head if my code will work or
not?)
jtoar pushed a commit that referenced this pull request Sep 6, 2023
We have this comment in `useToken()`


https://github.com/redwoodjs/redwood/blob/4b734d30c6830172194a2518ae0c2cbf6f1a0904/packages/auth/src/AuthProvider/useToken.ts#L7-L14

And for example our Netlify auth implementation does this:


https://github.com/redwoodjs/redwood/blob/4b734d30c6830172194a2518ae0c2cbf6f1a0904/packages/auth-providers/netlify/web/src/netlify.ts#L68-L78

But dbAuth let the exception slip through to `useToken`.
This PR adds a `catch` to the promise and returns `null`, just like
Netlify (and others)


Before merging this though, I need to make sure I'm not undoing the fix
@standup75 did in #8284
(@standup75 can you say off the top of your head if my code will work or
not?)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix This PR is a fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants