-
Notifications
You must be signed in to change notification settings - Fork 22
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
Collect user data and integrate error page with Intercom #1111
Conversation
This commit puts an ErrorBoundary around each Route's parent component. This way each ErrorBoundary is under the IntlProvider and has Intercom in scope so we integrate and prefill support request messages when crashes happen. ErrorBoundary component now gets a `component` prop to specify which component it is wrapping and this will be sent with the errbit report. It is also collecting user name, dbid and email to send with the report, to help gain more insight/context on the crashes. Note I: The @airbrake-js/browser calls to the Errbit server seem to ignore the extra data added as context, params and session, unless the error is a string or we cast a new instance from the raised error object. To be investigated. See github issue: airbrake/airbrake-js#1193 Note II: MainErrorBoundary has been renamed to ErrorBoundary Fixes: CHECK-1744
@@ -294,6 +294,9 @@ class MediaComponent extends Component { | |||
media.quote = media.media.quote; | |||
media.embed_path = media.media.embed_path; | |||
|
|||
// console.log(this.props.trololo.meajude.xocxoc); // eslint-disable-line no-console | |||
console.log(JSON.parse('bli')); // eslint-disable-line no-console |
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.
Stray console log!
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.
Thanks! Fixing this! Not only a console.log but one with a runtime error in it! 😅
id | ||
slug | ||
permissions |
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 see you moved slug
and permissions
into a subquery here, just making sure that was as intended. I don't see any code changes made in response to this move, which is why I ask, but maybe I'm missing one.
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 didn't. They're under team
all along. I guess it's the crazy diff visualization that may suggest it, but thanks for pointing!
@@ -64,8 +64,12 @@ function getStatusStyle(status, property) { | |||
* Truncate a string and append ellipsis. | |||
*/ | |||
function truncateLength(str, length = 70) { | |||
const dots = str.length > length ? '...' : ''; | |||
return `${str.substring(0, length)}${dots}`; | |||
if (typeof str === 'string') { |
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.
Ah nice catch, wouldn't want this attempting to substr
an Array
.
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.
Other than my comments above (specifically that console log), looks good!
Also to mention that change in helpers.js is to guard agains `null` value for `str`. Fixes CHECK-1747
params: { info: errorInfo }, | ||
session: { name, dbid, email }, | ||
}).then((response) => { | ||
if (Intercom) { |
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.
Didn't you need to declare "global Intercom" on top? The Linter didn't complain?
* Collect user data and integrate error page with Intercom This commit puts an ErrorBoundary around each Route's parent component. This way each ErrorBoundary is under the IntlProvider and has Intercom in scope so we integrate and prefill support request messages when crashes happen. ErrorBoundary component now gets a `component` prop to specify which component it is wrapping and this will be sent with the errbit report. It is also collecting user name, dbid and email to send with the report, to help gain more insight/context on the crashes. Note I: The @airbrake-js/browser calls to the Errbit server seem to ignore the extra data added as context, params and session, unless the error is a string or we cast a new instance from the raised error object. To be investigated. See github issue: airbrake/airbrake-js#1193 Note II: MainErrorBoundary has been renamed to ErrorBoundary Fixes: CHECK-1744 * Remove console log with intentional error 😅 Also to mention that change in helpers.js is to guard agains `null` value for `str`. Fixes CHECK-1747 * Add missing files 🤦
This commit puts an ErrorBoundary around each Route's parent component.
This way each ErrorBoundary is under the IntlProvider and has Intercom in scope so we integrate and prefill support request messages when crashes happen.
ErrorBoundary component now gets a
component
prop to specify which component it is wrapping and this will be sent with the errbit report.It is also collecting user name, dbid and email to send with the report, to help gain more insight/context on the crashes.
Note I: The @airbrake-js/browser calls to the Errbit server seem to ignore the extra data added as context, params and session, unless the error is a string or we cast a new instance from the raised error object. To be investigated. See github issue: airbrake/airbrake-js#1193
Note II: MainErrorBoundary has been renamed to ErrorBoundary
Fixes: CHECK-1744