Skip to content

Added default onError prop to <Dictation />#2866

Merged
tonyanziano merged 3 commits intomasterfrom
toanzian/dictation-err
Feb 5, 2020
Merged

Added default onError prop to <Dictation />#2866
tonyanziano merged 3 commits intomasterfrom
toanzian/dictation-err

Conversation

@tonyanziano
Copy link
Copy Markdown
Contributor

@tonyanziano tonyanziano commented Jan 28, 2020

Changelog Entry

  • Added default onError prop to the Dictation component, by @tonyanziano, in PR #2866

Description

Added a default onError prop to the <Dictation /> component so that errors caught by the <Dictation /> component will be surfaced in the console.

Specific Changes

Added a default onError prop to the <Dictation /> component so that errors caught by the <Dictation /> component will be surfaced in the console.

===

Let me know if didn't follow a pattern correctly. I'm not very familiar with hooks or the Web Chat code base.

@tonyanziano
Copy link
Copy Markdown
Contributor Author

tonyanziano commented Feb 4, 2020 •
edited
Loading

After some investigation, I think that my changes did not introduce a new error that is breaking the tests, but instead surfaced a preexisting error that is now breaking the tests.

It's hard to tell for sure without the ability to set breakpoints within the component package during test runs.

However, all my changes did was make it so that the <Dictation /> component calls console.error(<exception>) whenever there is an error with speech / dictation. The test that is breaking clicks the microphone button, and that makes sure that no errors were logged via console.error().

Before my changes, this test never failed because it was impossible for the component to call console.error() with an undefined error handler. However, now that the handler is defined, an error will cause the component to log a console error which then fails the test. The interesting part is that the error is just an empty string.

If I change my handler to do nothing, or to log to console.warn() / .info() / .log() then the test passes.

===

I was using useMemo() incorrectly. 😅

corinagum
corinagum approved these changes Feb 5, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@corinagum corinagum left a comment

Choose a reason for hiding this comment

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

LGTM. Sorry for the delay :)

@tonyanziano
Copy link
Copy Markdown
Contributor Author

tonyanziano commented Feb 5, 2020

@corinagum no worries, I didn't fix it until yesterday 😃

@tonyanziano tonyanziano merged commit 7efc568 into master Feb 5, 2020
@tonyanziano tonyanziano deleted the toanzian/dictation-err branch February 5, 2020 19:06
@compulim compulim mentioned this pull request Mar 5, 2020
Closed
40 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@a-b-r-o-w-n a-b-r-o-w-n Awaiting requested review from a-b-r-o-w-n a-b-r-o-w-n is a code owner

@compulim compulim Awaiting requested review from compulim compulim is a code owner

@cwhitten cwhitten Awaiting requested review from cwhitten cwhitten is a code owner

@srinaath srinaath Awaiting requested review from srinaath srinaath is a code owner

@tdurnford tdurnford Awaiting requested review from tdurnford tdurnford is a code owner

1 more reviewer

@corinagum corinagum corinagum approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tonyanziano @corinagum