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

Embedded error form not working when used with tunnel in JavaScript SDK #4679

Closed
kubijo opened this issue Feb 27, 2022 · 14 comments
Closed

Embedded error form not working when used with tunnel in JavaScript SDK #4679

kubijo opened this issue Feb 27, 2022 · 14 comments

Comments

@kubijo
Copy link

kubijo commented Feb 27, 2022

Environment

self-hosted (https://develop.sentry.dev/self-hosted/)

Version

22.2.0

Steps to Reproduce

  1. Use the latest official SDK with React & tunnel option
  2. Enabled embedded feedback dialog in ErrorBoundary component

Expected Result

  • Errors are reported through the tunnel endpoint (got this working through Django view, inspired by your Flask example)
  • Embedded form uses the tunnel endpoint to both load it's JS & submit data.

Actual Result

1) The URL at which the SDK looks for the embedded form's JavaScript is not configurable

I was able to go around this by fiddling with DSN value in options for the dialog, but that's an ugly work-around since DSN is at this point about identification of org/project as opposed to anything with API location.

2) Cannot configure the submit URL of embedded form

This is probably very much connected to the first problem, but even when I work around the first one, this one gets into my way… that is to say that I am able to get the form to show up, but it has the following additional problems:

2.1. Submit URL is based on the internal absolute URL

This doesn't work in the context of my application, since that URL won't be publicly available.

Also, as a sidenote, I'm sure it doesn't also need to include all the GET parameters that got there from dialog options (I'm supplying all texts because I'd like to have them be part of our translation catalogs.

var endpoint = /**/"http://127.0.0.1:9000/api/embed/error-page/?dsn=http%3A%2F%2Fe691f5a9f573484a95e8da0ee4fe55e2%40127.0.0.1%3A3010%2F2&title=It+looks+like+we%E2%80%99re+having+issues.&subtitle=Our+team+has+been+notified.&subtitle2=If+you%27d+like+to+help%2C+tell+us+what+happened+below.+%E2%80%93+not+visible+on+small+screen+resolutions&labelName=Name&labelEmail=Email&labelComments=What+happened%3F&labelClose=Close&labelSubmit=Submit&errorGeneric=An+unknown+error+occurred+while+submitting+your+report.+Please+try+again.&errorFormEntry=Some+fields+were+invalid.+Please+correct+the+errors+and+try+again.&successMessage=Your+feedback+has+been+sent.+Thank+you%21&eventId=cd203fb8d8934d5f8390a9b29cab7e6a";/**/ '';

2.2. The embed JS is injected twice & the form itself shows twice as a result.

This might quite possibly stem from the fact that development mode of react re-throws errors it catches, but as far I was able to gather, the ErrorBoundary component uses APIs from vanilla JS package.

There should be some kind of singleton sanity check around that JavaScript injection.


  • I can gather & supply more information upon request.
  • Even though the issue might seem like one with JavaScript/React SDK, the underlying logic is spread through the whole project, so I gathered that this repository should be the place to go.
@kubijo
Copy link
Author

kubijo commented Feb 28, 2022

As an alternative approach, I have gone the way of my own UI + API request to api/projects/submit-user-feedback, but now It's failing me in nothing being shown in the sentry UI… I am using the eventId attribute given to me in the fallback prop of Sentry.ErrorBoundary component…

It seems like the ID I am given doesn't match the error event ID that was sent to the backend… even the User Feedback for all envs & projects doesn't show anything even though I'm getting this response for the feedback API:

{
    "id": "19",
    "eventID": "f279b22d40614f6e8fc62750c29c489e",
    "name": "user_90000",
    "email": "[email protected]",
    "comments": "asdfggadgadfg",
    "dateCreated": "2022-02-28T12:23:36.850598Z",
    "user": null,
    "event": {
        "id": "f279b22d40614f6e8fc62750c29c489e",
        "eventID": "f279b22d40614f6e8fc62750c29c489e"
    },
    "issue": null
}

@kubijo
Copy link
Author

kubijo commented Feb 28, 2022

I have just forked your ErrorBoundary component to an internal one (one less abstraction) & got this piece of additional context:

  • Sentry.init / beforeSend(…)
    • 657fde8c04824d069ae9d52d15f9dc9b
  • @sentry/browser / captureException(…) (this is the same as you do in your ErrorBoundary)
    • 198657495c644dd888228885277d174a

Edit

Got those values to sync-up by commenting-out the GlobalHandlers Integration

@getsentry-release
Copy link

Routing to @getsentry/open-source for triage. ⏲️

@chadwhitacre
Copy link
Member

the underlying logic is spread through the whole project

Can you unpack this for me @kubijo? My instinct is to route this over to the sentry-javascript repo as it seems primarily to do with difficulty using the tunnel option with self-hosted (right?).

@kubijo
Copy link
Author

kubijo commented Mar 1, 2022

Let's try…

I will also state some presumptions and opinions held on my side in connection to the tunnel functionality so that you can approach this somewhat more systematically…

The way I see it, the tunnel is there to give us an option to pipe the sentry SDK through our server of choice & as such side-step several problems with dedicated sentry backend:

  • CORS
  • need to propagate credentials into the frontend code
  • need to have the sentry server exposed to internet (as opposed to, say, it being only accessible behind VPN)

Given these presumptions, my desire to have a React frontend connected to sentry and current state of affairs, I have to:

  • set up the SDK with a mocked DSN string so that it can instantiate
  • implement an API endpoint that:
    • proxies the calls into the sentry backend
    • intercepts the messages to replace the fake DSN in message headers with real one
  • disable default integrations (chiefly the GlobalHandlers one) & list a subset of them separately so that the captureException gives me a useful event ID (one to which I can pair-up user feedback)
  • since the JavaScript file for rendering the feedback modal assumes to be accessible under it's Django view URL (derived from sentry instance settings) & sends its results back to that URL, I have to:
    • fork the error boundary component because at this point it only gives me extraneous abstractions I am working against
    • render my own form
    • pipe its data back through a separate proxy endpoint because now I have to use a JSON API with a separate authentication mechanism (bearer token header with user-based API token)
    • understand & parse error cases from that API to show sensible user feedback

All that said, I'm aware that you cannot be sensibly expected to cover everyone's needs, but from my point of view there does seem to be a way to address all of these with few changes / additions that should break existing code.

  1. Allow instantiation of JavaScript SDK without DSN when used through tunnel
  2. Allow header-based authentication in the envelope API so that we don't have to touch the payload (if you get DSN from an HTTP header, don't expect it in the envelope header)
  3. Add a method to the JavaScript SDK to submit user feedback & pipe it through the envelope API (hence through the tunnel endpoint already implemented, no extra token with specific permissions)

As a sidenote to the point 3., you could both simplify the ErrorBoundary component & make it more flexible if you slightly modify its interface by adding that proposed method to the fallback callback as a new parameter

// New interface
type FeedbackPayload = {
    event_id: string
    user: string;
    email: string;
    comments: string;
};
type FeedbackStatus = {
    status: 'ok' | 'error';
    errors: Record<keyof FeedbackPayload, string>;
};
type SendFeedback = (FeedbackPayload) => Promise<FeedbackStatus>;
type SendFeedbackBound = (Omit<FeedbackPayload, 'event_id'>) => Promise<FeedbackStatus>;

// Slightly widened "ErrorBoundary.fallback"
type Fallback = 
    | ReactNode
    | (
        errorInfo: { error, componentStack, resetError },
        feedback: SendFeedbackBound, // Since you already know the event_id in the component
      ) => ReactNode;

However, I could take it or leave at this point, since the ErrorBoundary is really just a thin declarative wrapper around existing imperative APIs (good design, but I'm just connecting the dots here).

This design also presumes translated error messages in all supported languages on the customer web, but then again this presumption is there already & one way this could be addressed is returning some known set of IDs for the errors instead of error messages… or both and let users decide whether they want their own translations or use whatever comes …

@chadwhitacre
Copy link
Member

Okay yeah I'm gonna bump this to the JavaScript SDK team to digest that. 😂

Thanks for the great info!

@getsentry-release
Copy link

Routing to @getsentry/team-web-frontend for triage. ⏲️

@AbhiPrasad
Copy link
Member

This will be resolved once we send user feedback through envelopes instead of a dedicated endpoint. This will be unblocked by #4240

Moving to JS repo to mark as backlog + improvement.

@AuHau
Copy link

AuHau commented Jun 19, 2022

Is this then resolved since v7 is out?

@fisprak
Copy link

fisprak commented Jul 5, 2022

We use managed Sentry. We use the tunnel option but embedded error page is not loaded via tunnel. Embedded error form and crash reports are filtered out by EasyPrivacy list.

image

getBaseApiEndpoint doesn't take into account tunnel option.

@souredoutlook
Copy link
Member

Hey @smeubank - @fisprak and others are looking for an update on when submitting user feedback from the user report dialogue via tunnel will be supported.

@sebastianrath
Copy link

Hi, has there been any update on this? I've implemented a custom tunnel for the error handling, but the feedback dialog still tries to connect to sentry.io/api/embed/....

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 17, 2024
@mydea
Copy link
Member

mydea commented May 17, 2024

Hey,
have you tried looking into the new feedback dialog: https://docs.sentry.io/platforms/javascript/user-feedback/#set-up

This works differently now and can be embedded into your page better, which should work well with tunnelling too!

@mydea
Copy link
Member

mydea commented Jul 9, 2024

I am closing this issue because this can be solved with the new feedback UI. While this is not a 1-1 replacement, it is what we will continue to work on more in the future, so I think it makes sense to use this instead if needed!

@mydea mydea closed this as not planned Won't fix, can't repro, duplicate, stale Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests