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

Bug fix: Fixed Firebase promises so that errors/exceptions can be caught #2503

Merged
merged 3 commits into from
May 14, 2021

Conversation

cjreimer
Copy link
Contributor

This addresses #2461

Firebase promises cannot be caught with try { await } catch blocks, and thus all Firebase exceptions were unhandled, even if the user's application tried to catch the exception. A simple way to generate a Firebase exception is to try to sign on with an invalid e-mail address. I utilized some slight modifications to the playground-auth app for testing.

@peterp I'll tag you as you helpfully reviewed the issue I presented.

Comment on lines +30 to +33
// Firebase auth functions return a goog.Promise which as of 2021-05-12 does
// not appear to work with try {await} catch blocks as exceptions are not caught.
// This client returns a new standard Promise so that the exceptions can be
// caught and no changes are required in common auth code located in AuthProvider.tsx
Copy link
Contributor

Choose a reason for hiding this comment

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

Oooof, that's a good catch 🙌

Copy link
Contributor

@peterp peterp left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, this looks good to me.

@thedavidprice thedavidprice merged commit 47d5113 into redwoodjs:main May 14, 2021
@thedavidprice thedavidprice added this to the next-release milestone May 14, 2021
fveauvy pushed a commit to fveauvy/redwood that referenced this pull request May 15, 2021
Co-authored-by: David Price <thedavid@thedavidprice.com>
dac09 added a commit to dac09/redwood that referenced this pull request May 19, 2021
…-codegen

* 'main' of github.com:redwoodjs/redwood: (54 commits)
  Add private methods loose explicitly (redwoodjs#2554)
  Custom `useAuth` pass through for `RedwoodApolloProvider` (redwoodjs#2455)
  Prerender all routes nested in Set with prerender prop (redwoodjs#2542)
  Upgrade eslint and prettier packages including formatting fixes (redwoodjs#2540)
  contributors updates (redwoodjs#2544)
  Rename default datasource (redwoodjs#1941)
  Add default config for Component generation (redwoodjs#1814)
  build(deps): bump core-js from 3.10.1 to 3.12.1 (redwoodjs#2481)
  upgrade babel 7.14.2 with misc babel packages (redwoodjs#2541)
  build(deps): bump http-proxy-middleware from 1.1.0 to 2.0.0 (redwoodjs#2536)
  build(deps): bump pino-pretty from 4.7.1 to 4.8.0 (redwoodjs#2534)
  build(deps): bump concurrently from 6.0.2 to 6.1.0 (redwoodjs#2533)
  build(deps-dev): bump firebase-admin from 9.7.0 to 9.8.0 (redwoodjs#2522)
  build(deps): bump esbuild-loader from 2.10.0 to 2.13.0 (redwoodjs#2518)
  build(deps): bump @graphql-tools/merge from 6.2.13 to 6.2.14 (redwoodjs#2516)
  updating minor dependency versions across packages (redwoodjs#2532)
  Add JSON headers to Function generator template (redwoodjs#2457)
  fixed firebase promises so that they can be caught (redwoodjs#2503)
  Prevent re-initialization of a Firebase app that is already initialized (redwoodjs#2504)
  build(deps-dev): bump magic-sdk from 2.7.0 to 4.3.0 (redwoodjs#2463)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants