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(new-metrics): add TypeError #3527

Open
wants to merge 3 commits into
base: beta
Choose a base branch
from

Conversation

nataliadelmar
Copy link
Contributor

This pull request addresses

Right now, if a client metric event contains a TypeError error, it gets classified as 'BrowserMediaError' because TypeError is one of the possible error that can result from MediaDevices: getUserMedia() method (https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getUserMedia#aborterror:~:text=individual%20user%20agent.-,TypeError,-Thrown%20if%20the)

by making the following changes

That assumption is wrong, as any service can return a TypeError and, unfortunately, we do not have a way of identifying the service this error comes from. So this PR classifies it as a TypeError of category 'other'.

Change Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Tooling change
  • Internal code refactor

The following scenarios where tested

< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >

I certified that

  • I have read and followed contributing guidelines

  • I discussed changes with code owners prior to submitting this pull request

  • I have not skipped any automated checks

  • All existing and new tests passed

  • I have updated the documentation accordingly


Make sure to have followed the contributing guidelines before submitting.

@nataliadelmar nataliadelmar requested a review from a team as a code owner April 9, 2024 15:36
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-3527.d3m3l2kee0btzx.amplifyapp.com

@nataliadelmar nataliadelmar added the validated If the pull request is validated for automation. label Apr 9, 2024
Copy link
Contributor

@torgeadelin torgeadelin left a comment

Choose a reason for hiding this comment

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

some questions

* Any service can return a TypeError, which tells little about the service it came from
* Documentation can be found here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypeError/TypeError
*
* @param errorCode
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not really an error code

@@ -127,6 +126,7 @@ export const ERROR_DESCRIPTIONS = {
ICE_FAILED_WITH_TURN_TLS: 'ICEFailedWithTURN_TLS',
SDP_OFFER_CREATION_ERROR: 'SdpOfferCreationError',
SDP_OFFER_CREATION_ERROR_MISSING_CODEC: 'SdpOfferCreationErrorMissingCodec',
TYPE_ERROR: 'TypeError',
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you add it under SDP section? Is TypeError not a generic thing?

@sreenara
Copy link
Contributor

We are no longer accepting PRs on beta @nataliadelmar . Do we still need this PR? If not, can you please close this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
validated If the pull request is validated for automation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants