Repository navigation
[FIX] incorrect error toast messages - #26320
Merged
Merged
Conversation
* `handleError` handles all property checks and casting (if necessary)
debdutdeb
changed the title
debdutdeb
changed the title
Member
Author
|
ignore the very incorrect branch name 🙈 |
casalsgh
changed the title
casalsgh
changed the title
debdutdeb
changed the title
debdutdeb
marked this pull request as draft
July 20, 2022 19:26
debdutdeb
changed the title
debdutdeb
added
stat: needs QA
and removed
stat: ready to merge
`useEndpoint` throws a serialized object that doesn't implement `toString` which is causing `String(error)` to resolve to `[object Object]` even though the object contains the `error` property. On the other hand `handleError` checks for both `message` and `error` properties along with doing other things (and now defaults to a string conversion) so there is no need to check type on runtime and cast for each instance.
…t/Rocket.Chat into fix/undefined-error-toad-again
debdutdeb
marked this pull request as ready for review
July 26, 2022 17:53
Codecov Report
@@ Coverage Diff @@
## develop #26320 +/- ##
===========================================
+ Coverage 38.51% 38.65% +0.13%
===========================================
Files 758 758
Lines 18818 18820 +2
Branches 1461 1452 -9
===========================================
+ Hits 7247 7274 +27
+ Misses 11354 11328 -26
- Partials 217 218 +1
Flags with carried forward coverage won't be shown. Click here to find out more. |
tassoevan
previously requested changes
Aug 10, 2022
tassoevan
left a comment
tassoevan
left a comment
Member
There was a problem hiding this comment.
I screwed up big time in this one.
However, I don't like the idea of slapping any at the caught errors... I believe a better approach for it is introducing a new function that takes an unknown-type error and uses dispatchToastMessage under the hood to figure the "stringification" out.
…undefined-error-toad-again
|
This pull request introduces 1 alert when merging 11d3ffe into 50af9b2 - view on LGTM.com new alerts:
|
…undefined-error-toad-again
ggazzo
approved these changes
Aug 15, 2022
murtaza98
approved these changes
Aug 16, 2022
…undefined-error-toad-again
d-gubert
approved these changes
Aug 16, 2022
d-gubert
left a comment
d-gubert
left a comment
Member
There was a problem hiding this comment.
Looking good on the Apps-Engine side
csuadev
pushed a commit
that referenced
this pull request
Aug 26, 2022
Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
Merged
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Proposed changes (including videos or screenshots)
Issue(s)
https://open.rocket.chat/channel/rocketchat-dev?msg=NHaMAkZDYssEEBZW4
Steps to test or reproduce
Further comments
useEndpointthrows an object that doesn't implementtoStringwhich is causingString(error)to resolve to[object Object]even though the object contains theerrorproperty. On the other handhandleErrorchecks for bothmessageanderrorproperties along with doing other things (and now defaults to a string conversion) so there is no need to check type on runtime and cast for each instance manually.