Remix - No URL included with issues

[alexblack]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/remix

SDK Version

7.17.4

Framework Version

7.17.4

Link to Sentry event

https://sentry.io/organizations/syncwith/issues/3721867878/?project=5880196&referrer=issue-stream

Steps to Reproduce

1. We've integrated Sentry into our remix application, on the server and the client, and with express, as indicated in your instructions (hopefully we didn't make any mistakes)

2. When errors are reported in Sentry, I expected to be able to see the URL that the user was on when the error occured, and the user agent, and other information about the request, like we can with our node Sentry integrations.

See attached screenshot of a remix sentry error with no URL mentioned, and one from our node integration where we get the URL.

I believe we've integrated the express request handler as specified:

const { createRequestHandler } = require('@remix-run/express');
const { wrapExpressCreateRequestHandler } = require('@sentry/remix');

const createSentryRequestHandler =
  wrapExpressCreateRequestHandler(createRequestHandler);
 
  
app.all(
  '*',
  isProduction
    ? createSentryRequestHandler({ build: require('./build') })
    : (req, res, next) => {
        purgeRequireCache();
        const build = require('./build');
        return createSentryRequestHandler({ build, mode: MODE })(
          req,
          res,
          next
        );
      }
);

Expected Result

I expected that sentry would grab the URL and other relevant info from the current request and include it in the error. Note this error occured on the server (node) not client (react).

Actual Result

[alexblack]

All the request headers are missing too, and the user is not identified at all (whereas in Node the user is identified by IP address)

[AbhiPrasad]

Hey thanks for writing in @alexblack! Could you share your Sentry server initialization code?

@onurtemizkan mind taking a look at this when you get the chance?

[alexblack]

@AbhiPrasad thanks, sure on the server we do:

export const initSentry = () => {
  Sentry.init({
    beforeSend,
    dsn: SENTRY_ADDON_DSN,
    tracesSampleRate: 0.05,
    normalizeDepth: 6, // stringify deeper objects
  });
};

[onurtemizkan]

Hi @alexblack! Thanks for reporting.

I was not able to reproduce the issue with a generic Remix/Express project. Could you provide a minimal reproduction so we can debug it?

[alexblack]

Thanks for looking into this. Ok that's good to hear. Can you confirm - that it is expected then that the url, request headers etc, should show up in the server side issues? Do you know which part of the sentry remix config code enables this? Eg is there some chance I missed part of the integration?

…

On Tue, Nov 8, 2022, 4:17 AM Onur Temizkan ***@***.***> wrote: Hi @alexblack <https://github.com/alexblack>! Thanks for reporting. I was not able to reproduce the issue with a generic Remix/Express project. Could you provide a minimal reproduction so we can debug it? — Reply to this email directly, view it on GitHub <#6139 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABELQUURINJMWMA6SV6HUDWHJAE3ANCNFSM6AAAAAARYESTDY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[onurtemizkan]

@alexblack -- Yes, the URL is expected to show up in the server-side issues since version 7.17.1. (6c8d6c3)

It should work out of the box for document requests, actions and loaders. Only configuration I can think of that may cause this is the beforeSend implementation you are using.

But also, there may be a case in your project that we are missing to enrich captured events with RequestData integration. So, if that's the case, we'll need to reproduce and fix this.

[leandro-gomez]

Hi. I'm experiencing the same in my NestJS/Express application. No url, no headers, no user identified.
Versions 7.18 and 7.17 have the same problem, downgrading to version 7.16.0, however, restored those features.

Maybe this be related to this release, specifically to this PR?

[alexblack]

@onurtemizkan great, how about request headers? should I expect the user to be identified by IP address? Both of these work in node.

Here is a repo that I just created, using remix create, where I do see the URL (which is good) but I don't see any request headers, and the user is not identified by IP address (or at all).

https://github.com/alexblack/sentry-remix-test1

[lobsterkatie]

@leandro-gomez - You're right about the likely culprit. Unfortunately, the nextjs SDK has never fully supported the use of a custom server (which is what I assume you're doing with express), as many of our features are tied into the next server and how it works. That said, if you could provide a small repro case, it'd be easier to tell if this particular problem would be a small or big fix.

EDIT: Realized belatedly that we're talking about nest, not next.

[alexblack]

@onurtemizkan here are two issues and how to reproduce:

1. Pull down https://github.com/alexblack/sentry-remix-test1
2. replace DSH_HERE with your DSN
3. yarn && yarn build && yarn start
4. Browse to http://localhost:3000
5. See an error in the browser screen test
6. Find that error in sentry
7. See that Users is 0, expected 1
8. See that no request headers are shown (eg user agent, ip address, language, accepted content type etc)

Second issue (calling captureException inside withScope: https://github.com/alexblack/sentry-remix-test1/blob/main/app/lib/sentry.ts#L29)

1. Same as above to get started
2. Browse to http://localhost:3000/tutorial/foo
3. See error test5 on the screen
4. Find the issue in sentry
5. See that there is no URL shown (I expected to see the url populated and displayed as http://localhost:3000/tutorial/foo)
6. See that users is 0, expected 1
7. See no request headers shown

[leandro-gomez]

Hi @lobsterkatie

Here the repository: https://github.com/leandro-gomez/sentry-nestjs-6139

nvm use;
npm install;
env SENTRY_DSN="<key>" npm run start;

# wait for the server to start
curl localhost:3000 # this will trigger an exception

I've found the problem may be related to withScope.
Look at this commit: https://github.com/leandro-gomez/sentry-nestjs-6139/commit/1920bba27fc23ca66f22b1d6c17fcbc16f5827af

If you use captureException within withScope, you don't get all the information. But if you remove the withScope, all the information is there.

The Documentation says:

On the other hand, with-scope creates a clone of the current scope, and the changes made will stay isolated within the with-scope callback function. This allows you to more easily isolate pieces of context information to specific locations in your code or even call clear to briefly remove all context information.

[lobsterkatie]

Thanks, @leandro-gomez.

If you use captureException within withScope, you don't get all the information. But if you remove the withScope, all the information is there.

Interesting. We must not be retrieving the correct scope when we go to clone it.

EDIT: Oh, jeez. I just realized when I looked at your repro that this is a neSt app, not a neXt app. We don't have official nest support (and I'm not super familiar with nest), but my guess would be that the pipe maybe breaks you out of the request's domain and so we lose scope data. It worked with the withScope before the express change?

[leandro-gomez]

It worked with the withScope before the express change?

NestJS uses Express internally. We were using Sentry with NestJS just fine until recently.

[lobsterkatie]

Right, I understand. The thing is, the event processor which used to add the data also lived on the scope, so I'd think that if withScope gets the wrong scope to clone, the old way wouldn't have worked, either. So I'm just trying to confirm that you have been using withScope the whole time.

Perhaps we can back up a step, though: Is the fact that you're manually capturing the error (with or without withScope) an artifact of this being a toy example, or do you do that in prod also? Is our errorHandler incompatible with nest?

[leandro-gomez]

Yes, We were using withScope the whole time, from months now.

do you do that in prod also? Is our errorHandler incompatible with nest?

This is how it's configured in production, the errorHandler doesn't work with NestJS

[alexblack]

hi @onurtemizkan did those steps and github repo give you what you need to look into this? thanks

[alexblack]

hi @onurtemizkan just following up, thanks!

[onurtemizkan]

Hi @alexblack, thanks for reproductions! I have been debugging them, and there seem to be a few things here we need to figure out.

cc: @lobsterkatie

• Remix uses its own Fetch API implementation on both back-end and front-end, which we are not parsing correctly. (For example, headers property is behind a Proxy object unlike Node requests). So we are not able to extract data correctly. Need to open a PR to fix this.

• Using an Express adapter (Express has its own request implementation that we support) doesn't solve the problem above completely, as in current implementation, Express wrapper is no-op if auto-instrumentation of core createRequestHandler succeeds. So we need to make sure everything works in the same manner, regardless of what gets instrumented.

• The second issue about manual instrumentation seems to be primarily about the usage of withScope. You are losing the scope context. But after all, I think it depends on the first issue, so we should figure that out first, IMHO.

Also, as a side note, there is some missing configuration on your side about connecting FE and BE services. So your baggage and sentry-trace headers are not assigned properly. Fixing this won't resolve the issue in general, but will probably improve your workflow while we're working on this.

[alexblack]

Thanks for looking into all this. And thanks for the tip, I'll look into the config to connect front end and backend.

[lobsterkatie]

@leandro-gomez Got it, thanks.

UPDATE: I'd written a whole long thing here, and in the process of writing the last sentence, it all of a sudden dawned on me what the problem was. Should be fixed in #6218. Yay for rubber ducks.

[lobsterkatie]

@onurtemizkan - So I think that of your three points listed above, only the second is outstanding, yeah?

[alexblack]

Hi @AbhiPrasad I re-ran the repro steps with sentry 7.20.1, and in both cases I get 0 users, when I expected 1. Am I missing something?

See: https://develop.sentry.dev/sdk/data-handling/

Sentry server is always aware of the connecting IP address and can use it for logging in some platforms. Namely JavaScript and iOS/macOS/tvOS. All other platforms require the event to include user.ip={{auto}} which happens if sendDefaultPii is set to true.

I think its expected that there is a user identified in each of these errors, since these errors happen on the server. Regardless, I tried setting sendDefaultPii: true in the initialization on both server and client in the repo, and that didn't change anything, I still got 0 users for each error, when I expected 1

(Everything else looks much improved, URLs show up now, headers show up now, etc, thanks!)

[alexblack]

Also, it seems like transactions are not parameterized as expected

For the URL http://localhost:3000/tutorial/foo I expected the transaction to be something like GET https://localhost:3000/tutorial/$slug instead of GET https://localhost:3000/tutorial/foo, is this something that sentry is meant to do? I think you do something like this for node/express (without remix)

[AbhiPrasad]

I think its expected that there is a user identified in each of these errors, since these errors happen on the server. Regardless, I tried setting sendDefaultPii: true in the initialization on both server and client in the repo, and that didn't change anything, I still got 0 users for each error, when I expected 1

You have to explicitly enable this by setting include.ip: true. See: https://docs.sentry.io/platforms/node/configuration/integrations/default-integrations/#requestdata

I expected the transaction to be something like GET https://localhost:3000/tutorial/$slug instead of GET https://localhost:3000/tutorial/foo, is this something that sentry is meant to do? I think you do something like this for node/express (without remix)

We should be parameterizing, @onurtemizkan could you confirm that this works?

[alexblack]

Thanks I'll try the IP setting. Regarding parametrization, you can see with the GitHub repo and reproduction steps that I provided (above) that it's not working in this case.

…

On Tue, Nov 22, 2022, 4:25 AM Abhijeet Prasad ***@***.***> wrote: I think its expected that there is a user identified in each of these errors, since these errors happen on the server. Regardless, I tried setting sendDefaultPii: true in the initialization on both server and client in the repo, and that didn't change anything, I still got 0 users for each error, when I expected 1 You have to explicitly enable this by setting include.ip: true. See: https://docs.sentry.io/platforms/node/configuration/integrations/default-integrations/#requestdata I expected the transaction to be something like GET https://localhost:3000/tutorial/$slug instead of GET https://localhost:3000/tutorial/foo, is this something that sentry is meant to do? I think you do something like this for node/express (without remix) We should be parameterizing, @onurtemizkan <https://github.com/onurtemizkan> could you confirm that this works? — Reply to this email directly, view it on GitHub <#6139 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABELQQ6HT3XCBOYUW2RMX3WJS3SFANCNFSM6AAAAAARYESTDY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[alexblack]

Ok I tried using include.ip: true. This appears to be in the RequestData integration, available in @sentry/node. I modified the github repo (above) to include this in entry.server.tsx:

import * as SentryNode from "@sentry/node";

Sentry.init({
  dsn: "DSN_HERE",
  tracesSampleRate: 1,
  beforeSend,
  sendDefaultPii: true,
  integrations: [
    new SentryNode.Integrations.RequestData({
      include: {
        ip: true
      }
    }),

  ]
});

I then followed the reproduction steps I provided above, and same issue, Users is still 0, when I expected it to be 1.

[alexblack]

Hi, I ran the repro steps again today with that github repo, using 7.22.0 (released today), and I still see the same issue.
Are you guys seeing something different when you test it?

Sentry reports 0 users for each error, when I expected it to report 1 user, given that the repo initializes sentry on the server like this:

Sentry.init({
  dsn: "dsn_here",
  tracesSampleRate: 1,
  beforeSend,
  sendDefaultPii: true,
  integrations: [
    new SentryNode.Integrations.RequestData({
      include: {
        ip: true
      }
    }),

  ]
});

Is there something else I need to do to get Sentry to identify the user by IP address here?

[alexblack]

Hi, any update? I am still not seeing users identified by Sentry here. I reported this in detail in the repro steps on Nov 10th: #6139 (comment) and mentioned it earlier on Nov 5th: #6139 (comment)

It seems like Sentry's documentation says you support this, I've mentioned it many times in this thread, it seems like its not working. Are you seeing something different when you test it?

[lforst]

@alexblack Hi, we tried to resolve this in https://github.com/getsentry/sentry-javascript/pull/6263/files. As you can see, remix doesn't provide a way to read an IP off a request. Our attempted fix is using a best-effort heuristic to determine the IP based on incoming headers that might contain the user ip. If none of these headers are on the request, there's no way to determine the IP.

The express integration can read the IP off the request object because express puts it there. However, that way of reading the IP has its own set of problems, since it can't always be trusted or it might even be wrong depending on the networking setup.

Of course, we could set the user to a random ID just so that "users: 0" disappears, but that would be about as useful as having "users: 0".

One thing I could think of would be to provide a hook in the SDK options that would allow you to set the user yourself depending on what's on the request object remix provides. There you could use a (hashed) session cookie or really anything that's on there to determine a user value. Would something like that help in your case?

The remix SDK is still quite young and there are things to improve so thank you for your feedback!

[alexblack]

Hi @lforst I get that finding the IP address might be challenging, but, we see the IP address on every server side sentry event in the headers, using a header name that remix-utils/getClientIpAddress checks for.

You guys closed this issue several times. Each time you closed the issue, was the problem fixed? eg in your test was Sentry/remix now able to identify users by IP address?

Remix has a function getClientIpAddress in remix-utils I imagine would work: https://github.com/sergiodxa/remix-utils/blob/main/src/server/get-client-ip-address.ts#L12

See this issue in production using latest: https://sentry.io/organizations/syncwith/issues/3780781591/?project=5880196&query=is%3Aunresolved&referrer=issue-stream

[AbhiPrasad]

We added this utility function for the functionality that initially closed this issue:

sentry-javascript/packages/remix/src/utils/getIpAddress.ts

Line 36 in 7502677

'CF-Connecting-IP',

For some reason though, it doesn't seem to be getting detected, so we'll keep investigating here.

[alexblack]

Here's another example: https://sentry.io/organizations/syncwith/issues/3782138918/?project=5880196&query=is%3Aunresolved&referrer=issue-stream

In this example, I'm using the remix-utils function getClientIpAddress in beforeSend to put the IP address into the event, in the additional data section, and it works fine.

But, sentry still says 0 users, despite the fact that the IP address is available easily.

Before send:

export const beforeSend = (
  event: Event,
  hint: EventHint
): PromiseLike<Event | null> | Event | null => {

    return {
      ...event,
      extra: {
        'IP Address': event.request?.headers
          ? getClientIPAddress(createRemixHeaders(event.request.headers))
          : undefined,
        ...event.extra,
      },
    };
};

Helper function:

export function createRemixHeaders(
  requestHeaders: express.Request['headers']
): Headers {
  const headers = new Headers();

  for (const [key, values] of Object.entries(requestHeaders)) {
    if (values) {
      if (Array.isArray(values)) {
        for (const value of values) {
          headers.append(key, value);
        }
      } else {
        headers.set(key, values);
      }
    }
  }

  return headers;
}

[onurtemizkan]

Hi @alexblack, just tried to reproduce with the project you provided. It's working fine on local (with manually added headers).

The implementation of getClientIpAddress is almost identical in remix-utils and here, except isIP function, which we import from net core module. So I wonder if net core module is available in your runtime environment. Are you able to reproduce the issue on your local or another deployment target?

[alexblack]

Hi @onurtemizkan sure thing I'll try deploying the reproduction repo and get back to you.

If the net core module was missing, wouldn't I expect to see error(s) related to that if you're using that?

It looks to me like this module is part of node, so I'm not sure how we could be missing it. We're using node and express.

[alexblack]

Hi @onurtemizkan I deployed the repo here: https://sentry-remix-test1.onrender.com/

And I still have the same issue, users is at 0, despite the IP address being in the headers.

[alexblack]

Ah my mistake, it looks like the github repo is not yet using the latest sentry... I am going to update it, redeploy and try again.

[alexblack]

Ok my apologies, its working now in this test repo. Great!

Its still not working for me in our production app, but I realized I am not yet using the RequestData integration there, I'll add that and hopefully that does it.

Thanks for your help.

[alexblack]

This is working now, thanks!

[alexblack]

One question - it seems like what was missing was using the RequestData integration to specify to include the ip. This integration is in sentry/node, which seems to imply its just on the server.

Do I need to do anything to get this functionality on the client? eg if an error happens on the client, and is tracked to sentry, will the user be identified by IP address?

[lforst]

Do I need to do anything to get this functionality on the client? eg if an error happens on the client, and is tracked to sentry, will the user be identified by IP address?

For the client our ingest servers will pick up the IP address the event got sent from. So it should just work there.

[AbhiPrasad]

Going to close since this seems fine, but please reach out if there are any other problems.