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

Add support for API timeouts to auth-next #2915

Merged
merged 3 commits into from Apr 16, 2020

Conversation

avolkovi
Copy link
Contributor

Hey there! So you want to contribute to a Firebase SDK?
Before you file this pull request, please read these guidelines:

Discussion

  • Read the contribution guidelines (CONTRIBUTING.md).
  • If this has been discussed in an issue, make sure to link to the issue here.
    If not, go file an issue about this before creating a pull request to discuss.

Testing

  • Make sure all existing tests in the repository pass after your change.
  • If you fixed a bug or added a feature, add a new test to cover your code.

API Changes

  • At this time we cannot accept changes that affect the public API. If you'd like to help
    us make Firebase APIs better, please propose your change in an issue so that we
    can discuss it together.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 15, 2020

Binary Size Report

Affected SDKs

No changes between base commit (11dd5ee) and head commit (ef1d48a).

Test Logs

packages-exp/auth-exp/src/api/index.test.ts Outdated Show resolved Hide resolved
packages-exp/auth-exp/src/api/index.ts Outdated Show resolved Hide resolved
packages-exp/auth-exp/src/api/index.ts Outdated Show resolved Hide resolved
@@ -88,7 +92,8 @@ export async function performApiRequest<T, V>(
} else {
// TODO probably should handle improperly formatted errors as well
// If you see this, add an entry to SERVER_ERROR_MAP for the corresponding error
throw new Error(`Unexpected API error: ${json.error.message}`);
console.error(`Unexpected API error: ${json.error.message}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this meant to be here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the console.error since we never expect to see these, it would help in debugging since we mask json.error.message with an error from the map (in this case, internal-error)

@avolkovi avolkovi force-pushed the avolkovi/auth-next-api-timeout branch from 6021352 to b8e5442 Compare April 15, 2020 22:27
@avolkovi avolkovi force-pushed the avolkovi/auth-next-api-timeout branch from b8e5442 to 07ac4b2 Compare April 16, 2020 17:39
@avolkovi avolkovi merged commit 5e70dac into auth-next Apr 16, 2020
@avolkovi avolkovi deleted the avolkovi/auth-next-api-timeout branch April 16, 2020 18:07
avolkovi added a commit that referenced this pull request Apr 22, 2020
* Add support for API timeouts to auth-next

* PR feedback

* [AUTOMATED]: Prettier Code Styling
avolkovi added a commit that referenced this pull request Apr 23, 2020
* Add support for API timeouts to auth-next

* PR feedback

* [AUTOMATED]: Prettier Code Styling
avolkovi added a commit that referenced this pull request Apr 24, 2020
* Add support for API timeouts to auth-next

* PR feedback

* [AUTOMATED]: Prettier Code Styling
@firebase firebase locked and limited conversation to collaborators May 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants