This repository has been archived by the owner on Oct 12, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 22
Report errors to the server when the DefaultError is thrown #673
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Check if the there is no code associated with the error and if so, send the error and detials that exist to the server.
jessgusclark
changed the title
[DRAFT] Error reporting
[DRAFT] Report errors to the server when the DefaultError is thrown
May 6, 2021
jessgusclark
changed the title
[DRAFT] Report errors to the server when the DefaultError is thrown
Report errors to the server when the DefaultError is thrown
May 6, 2021
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.
LGTM, few minor suggestions only
itofarina
reviewed
May 6, 2021
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'm just not sure about reporting the errors conditionally inside catch
blocks, I might be missing something but I feel like if something was caught, then we should report it.
Apart from that, this PR is great and LGTM :)
…e it where the DefaultError may be called.
itofarina
approved these changes
May 10, 2021
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.
LGTM 👌
patogallaiovlabs
approved these changes
May 10, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rational
Most of the errors this will be used with will be called from code files, however, some do occur within components. Until the code is separated, this module should be able to be called from anywhere.
It allows the developer to send optional parameters
developerComment
andadditionalInfo
to provide information that may not be in the error.developerComment
could be the location of the error, or part of the process, or something where the developer could search for the string in the code and find the location quickly.additionalInfo
could be the address of the user, or the subdomain they are trying to register, or any other information that would be helpful.Initial triggers
In this initial batch of errors, they are triggered where the getDefaultError() is found. This is the error that shows the user 'Internal server error'.
It also will send a report in a few places where an error was caught and then console.log out. These errors may be caught at a higher level and not needed, but that is anyone's guess.
wanna test it?
There is a sister branch called
error-reporting-manual-trigger
that you can clone and run. Navigate to the "Me" section, and under "settings" the first item will be 'Internal Server Error' or the equivalent in your language. Click it! It will send an error to the server.This sister branch should not be merged due to this commit.
sending an error
The developer can send an error like this:
This creates an error report with the following structure:
I am unsure if the stack trace will be helpful. My thinking is that we could put identifying information in the
developerComment
section that would find the error quickly.If there is an
error.code
, as many of the server calls have, that will also be included in theerrorObject
.future
In the future, an error could be reported by the user, or a modal could be created with an input box for the user to add additional details. This could be a new parameter in the object that is sent.