Skip to content
This repository was archived by the owner on Jan 5, 2026. It is now read-only.

Added circular JSON handling to forwardToMain #2242

Merged
tonyanziano merged 4 commits into from
Mar 9, 2021
Merged

Added circular JSON handling to forwardToMain #2242

tonyanziano merged 4 commits into from
Mar 9, 2021

Conversation

@tonyanziano
Copy link
Contributor

@tonyanziano tonyanziano commented Mar 8, 2021

When connecting to a Direct Line Speech enabled bot, the Web Chat speech adapter object embedded in one of the redux payloads contained a circular JSON structure -- privReco property -- that was causing the JSON.stringify() call in forwardToMain.ts to fail.

This was ultimately causing the Redux action to fail, which resulted in an undefined speech adapter object being sent to Web Chat. Web Chat could not complete the connection to the DL Speech channel.

This PR adds error handling for this specific case, and will bypass forwarding the speech adapters update action over to the main process of the app. It is not needed in the main process's state store anyways.

All other errors will be thrown like normal.

As an example, here's what the error message looked like:

Converting circular structure to JSON
    --> starting at object with constructor 'p'
    |     property 'privReco' -> object with constructor 'D'
    --- property 'privRecognizer' closes the circle"

@coveralls
Copy link

coveralls commented Mar 8, 2021 •
edited
Loading

Coverage Status

Coverage decreased (-0.02%) to 67.823% when pulling 6a89d4a on toanzian/speech into 860b5f2 on main.

corinagum
corinagum approved these changes Mar 9, 2021
View reviewed changes
Copy link
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 - just one question

@tonyanziano tonyanziano merged commit e879409 into main Mar 9, 2021
@tonyanziano tonyanziano deleted the toanzian/speech branch March 9, 2021 21:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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.

4 participants

@tonyanziano @coveralls @corinagum