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.

Sign up for GitHub

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

Jump to bottom

fix: call onError in init function #368

Merged
merged 3 commits into from Mar 12, 2021
Merged

fix: call onError in init function #368

merged 3 commits into from Mar 12, 2021

Conversation

ajhorst
Copy link
Contributor

@ajhorst ajhorst commented Mar 11, 2021

Summary

Use SDK consumer-supplied onError function throughout the codebase and make sure it is properly invoked if an error occurs during initialization

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?: No

AJ Horst added 2 commits March 11, 2021 14:57
AMP-32671 read in functions from options arg to amplitude client options
cdecaf0
AMP-32671 call onError from args in amplitude client initialization
28856c8 
- the client's options may not have been initialized when an error occurs, so call onError from the args to make sure it's invoked regardless of when the error is thrown
@ajhorst ajhorst changed the title fix() init onerror fix: call onError in init function Mar 11, 2021
kelvin-lu
kelvin-lu approved these changes Mar 12, 2021
View reviewed changes
Copy link
Contributor

@kelvin-lu kelvin-lu left a comment

Choose a reason for hiding this comment

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

lgtm! was about to ask for unit tests but you put em in 🎉

jooohhn
jooohhn approved these changes Mar 12, 2021
View reviewed changes
Copy link
Contributor

@jooohhn jooohhn left a comment

Choose a reason for hiding this comment

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

👍

@ajhorst ajhorst merged commit d2ae868 into main Mar 12, 2021
@ajhorst ajhorst deleted the AMP-32671_init_onerror branch March 12, 2021 01:17
github-actions bot pushed a commit that referenced this pull request Mar 15, 2021
@amplitude-sdk-bot
chore(release): 8.0.0 [skip ci]
03a331d 
# [8.0.0](v7.4.4...v8.0.0) (2021-03-15)

### Bug Fixes

* call onError in init function ([#368](#368)) ([d2ae868](d2ae868))
* remove callback from argument to function that takes no arguments ([#370](#370)) ([777dead](777dead))
This was referenced Mar 16, 2021
Merged
Merged
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jooohhn jooohhn jooohhn approved these changes

@kelvin-lu kelvin-lu kelvin-lu approved these changes

@kelsonpw kelsonpw Awaiting requested review from kelsonpw

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ajhorst @jooohhn @kelvin-lu