Skip to content
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

Fix: possible errors on rocket.chat side of the apps #10252

Merged
merged 3 commits into from
Mar 28, 2018

Conversation

graywolf336
Copy link
Contributor

These errors were added to ensure the data to the Rocket.Chat Apps system could be guaranteed, however this sadly can not be the case as Rocket.Chat doesn't have internal data verification and ensurations.

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-10252 March 28, 2018 15:45 Inactive
@@ -50,7 +50,7 @@ export class AppUsersConverter {
case 'bot':
return UserType.BOT;
default:
throw new Error('Unknown user type of:', type);
return type.toUpperCase();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep spitting out at least a warning?

To at least make sure we are always aware that we have types that aren't valid?

We should be striving for default to never be hit. If we have to do so by being annoying to our self maybe not such a bad thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some warning logs now. 👍

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-10252 March 28, 2018 17:14 Inactive
@rodrigok rodrigok changed the title [OTHER] Fix possible errors on rocket.chat side of the apps Fix: possible errors on rocket.chat side of the apps Mar 28, 2018
@rodrigok rodrigok added this to the 0.63.0 milestone Mar 28, 2018
@rodrigok rodrigok merged commit 761d84e into develop Mar 28, 2018
@rodrigok rodrigok deleted the prevent-errors-on-rocketchat-side-to-apps branch March 28, 2018 18:45
@rodrigok rodrigok mentioned this pull request Apr 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants