-
Notifications
You must be signed in to change notification settings - Fork 750
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
Thrown errors in client sagas are now logged to console. #2087
Conversation
bffe200
to
3126dd1
Compare
@@ -50,13 +50,18 @@ export function* throwErrorFromResponse(errorMessage: string, response: Response | |||
status, | |||
}; | |||
if (text) { | |||
const errText = yield text(); | |||
const errText = yield text.call(response); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was also throwing the following error:
TypeError: Failed to execute 'text' on 'Response': Illegal invocation
because text()
was being called outside of the response
context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good for now. But we just do a try catch block around all our sagas and log them.
@@ -50,13 +50,18 @@ export function* throwErrorFromResponse(errorMessage: string, response: Response | |||
status, | |||
}; | |||
if (text) { | |||
const errText = yield text(); | |||
const errText = yield text.call(response); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good for now. But we just do a try catch block around all our sagas and log them.
@srinaath once we bump |
This will help debug issues like #2086
===
Due to the way sagas work, the errors thrown in our nested sagas are seen as uncaught and redux saga hides most of the internal properties of the thrown error and just shows the
message
, which isn't very helpful.redux-saga
provides anonError
hook that you can use for catching these errors at the root level, but theonError
will only get called if the caught error is of typeError
. Since we defined our own error typeResponseError
theonError
hook never gets called. We need to update toredux-saga@1.0.0
for that check to be fixed, but this might incur some other breaking changes that I don't think this PR should deal with. See this issue for reference.For now, we will surface the error in its entirety through
console.error()
.Before the change:
After the change: