sentry-module icon indicating copy to clipboard operation
sentry-module copied to clipboard

Possible to enable User Feedback feature with this module?

Open darrenchiu opened this issue 2 years ago • 7 comments

Is your feature request related to a problem? Please describe.

No

Describe the solution you'd like

I would like to enable the User Feedback feature. Is this possible with this module? https://docs.sentry.io/platforms/javascript/guides/vue/enriching-events/user-feedback/

Describe alternatives you've considered

darrenchiu avatar Jan 29 '22 03:01 darrenchiu

It would have to be done using the beforeSend hook, as shown in one of the examples in the documentation but it won't work currently because there is no way to access the Sentry Browser export within that function.

I think that to support that, it would have to be possible to provide a file path as clientConfig configuration. Nuxt does stringify on configs so that messes any attempts to import stuff within those functions.

rchl avatar Jan 30 '22 10:01 rchl

It would have to be done using the beforeSend hook, as shown in one of the examples in the documentation but it won't work currently because there is no way to access the Sentry Browser export within that function.

I found that you can do it by accessing $sentry on the global window.$nuxt object, like:

// nuxt.config.js

  sentry: {
    clientConfig: {
      beforeSend(event) {
        const Sentry = window?.$nuxt?.$sentry;

        // Check if it is an exception, and if so, show the report dialog
        if (event.exception && Sentry?.showReportDialog != null) {
          Sentry.showReportDialog({ eventId: event.event_id });
        }

        return event;
      },
    },
  },

sgarner avatar May 18 '22 21:05 sgarner

A little hacky for my liking but true, it's an option.

But probably I would just do a truthy check Sentry?.showReportDialog instead of Sentry?.showReportDialog != null as we don't want to call the method in case it's undefined.

rchl avatar May 18 '22 21:05 rchl

A little hacky for my liking but true, it's an option.

Agree it would be great to come up with a better solution, but this hack could help some people in the meantime.

But probably I would just do a truthy check Sentry?.showReportDialog instead of Sentry?.showReportDialog != null as we don't want to call the method in case it's undefined.

That is covered because null == undefined, so a != null condition matches both. Otherwise, whether you want to use truthiness to check for existence of a property is a matter of personal preference I guess. 🙂

sgarner avatar May 18 '22 22:05 sgarner

That is covered because null == undefined, so a != null condition matches both.

Fair but that's IMO bad practice since you are seemingly checking against a specific value but really it does something else. That's why in all projects I work on only strict comparison operator is allowed to avoid this ambiguity.

rchl avatar May 18 '22 22:05 rchl

in all projects I work on only strict comparison operator is allowed to avoid this ambiguity

In projects I work on the use of strict comparison operators is generally enforced for the same reason: loose equality can be dangerous. But checks for existence of a value are an exception to the rule because the semantics of == null are clear and the alternative of writing (value === null || value === undefined) is overly verbose.

If the intention of a line of code is to check for existence and instead you check for truthiness then ambiguity and uncertainty about the type of the expected value are introduced (is this property actually a boolean?). When you write if (property) you're really writing a shorthand for if (property == true), which is again a loose equality comparison. When expecting a function this may not matter, but in other circumstances you have to be aware of the potential for subtle bugs in circumstances where the property could have a falsy value like 0 or "". That's why I prefer to check for existence explicitly.

Anyway, we're getting pretty far into the weeds here... 🌱

sgarner avatar May 18 '22 23:05 sgarner

Personally I'd prefer to use loose truthiness check if (property) then something that looks almost like a strict check but isn't (if (property != null)).

Of course if I'm checking if method exists a better option might be if (typeof(property) === 'function') anyway.

Just my 5 cents but I agree we can leave it at that :)

rchl avatar May 19 '22 07:05 rchl