[Bug]: `FetchPlugin` does not support server-side rendering

quisido commented 1 year ago

Which web client version did you detect this bug with?

v1.12.0

What environment (build systems, module system, and framework) did you detect this bug with?

NextJS 12.3.4
React 18.2.0
TypeScript 4.9.5
Webpack (unknown version, NextJS internal)

Is your web application a single page application (SPA) or multi page application (MPA)?

SPA

Please provide your web client configuration

N/A

Please describe the bug/issue

When instantiating new AwsRum(), server-side rendering throws an error:

TypeError: Failed to fetch
     at VirtualPageLoadTimer._this.fetch (webpack-internal:///./node_modules/aws-rum-web/dist/es/sessions/VirtualPageLoadTimer.js:57:18)
     at eval (webpack-internal:///./node_modules/aws-rum-web/dist/es/sessions/VirtualPageLoadTimer.js:71:33)
     at FetchPlugin._this.fetch (webpack-internal:///./node_modules/aws-rum-web/dist/es/plugins/event-plugins/FetchPlugin.js:163:18)
     at eval (webpack-internal:///./node_modules/aws-rum-web/dist/es/plugins/event-plugins/FetchPlugin.js:179:33)

It appears to be trying to call window.fetch once instantiated, which of course does not exist.

I would like to instantiate new AwsRum on the server without it actually connecting to emit any metrics, since there is no user in this context -- there should be nothing to emit, as no events have been triggered.

I understand that a server-side rendered page cannot (should not) be connecting to RUM to emit metrics.
My concern is that I do not believe that instantiating AwsRum in and of itself should not be connecting to RUM to emit metrics.

Please advise. 😊

adebayor123 commented 1 year ago

Hi Charles, web clients should not be initialized on a server - is there a specific reason why you wish to do so?

quisido commented 1 year ago

I'm server rendering a React application, so the client side runtime code is also executing on the server.

hjniermann commented 1 year ago

Hi, does this issue mean that RUM is not supported when server-side rendering is enabled on frameworks such as NextJS? This should be clearly noted, as NextJS is currently the recommended React framework, and is a blocker to teams using server-side rendering. @CharlesStover , did you find a way around this for your use case?

quisido commented 1 year ago

You can use useEffect to instantiate RUM only after your application has loaded on the client, even though this is an anti-pattern per NextJS.

You can use patch-package to patch the RUM package to not call window.fetch on the server.

I forget which path I ended up going.

qhanam commented 1 year ago

When initialized, the web client (1) performs steps to get credentials, and (2) emits a session start event. I believe both of these events can be deferred.

Regarding (2), initialize the web client in the disabled state by setting enabled: false in the config. The web client will not dispatch events when disabled.

This setting is currently missing from the docs, which we should fix. The use of this setting is demonstrated in this blog.

Regarding (1), omit the guestRoleArn and identityPoolId from the config. When the application is ready, use the setAwsCredentials command to pass the credential provider.

We may want to do something so that credentials are not fetched when the web client is disabled, in which case the application need only flip the web client from disabled to enabled when it is ready.

qhanam commented 1 year ago

Please re-open if this doesn't work or if you have thoughts on a better solution.

Meanwhile, we'll add these to the backlog.

quisido commented 1 year ago

I'll give those a try. As a placeholder, I've seen as a common pattern for SSR+CSR packages to use if (typeof window === 'undefined') to determine if they are rendering on a client or server. This pattern may work for you if you want RUM to no-op on a server render: only call window.fetch if window is defined.

qhanam commented 1 year ago

So to clarify, when rendering on a server (i.e., no window object), you expect all calls to the web client to be no-ops?

quisido commented 1 year ago

So to clarify, when rendering on a server (i.e., no window object), you expect all calls to the web client to be no-ops?

Yes, server-rendering serves primarily to generate the initial DOM. Once passed to the client, React will "hydrate," re-executing the render to (1) build the virtual DOM into memory [since the actual DOM is already present] and (2) execute any non-DOM side effects. For example, instantiating RUM and monitoring web vitals does not impact the initial DOM or view, so this behavior does not need to occur during the initial server render. There's no value in having a RUM object in server memory, but once the application re-hydrates on the client, RUM can instantiate in memory and perform browser-based actions like fetching and web vitals.

For a server-side rendered application, the application code is executed once on the server to build the initial DOM and again on the client to hydrate the application into browser memory. By saying "only perform this actions when the window object exists," it will no-op on the server, but it will still execute in the browser during the hydration cycle of the application, as that same JavaScript will re-execute, however now in an environment where window exists.

On the server (or any environment where window does not exist), I don't think it makes sense to call a metric "real user monitoring," because it would not be emit by a user -- it could be a server or a test runner in case of unit testing. No-op makes sense to me to prevent machine-generated metrics from being included as "real users," as there is value in having machines execute the RUM runtime code while traversing code paths of the parent application.

For power user inversion of control, you could perhaps support fetch as a parameter, but I'm really scope-creeping with this.

class RUM {
  private _fetch;
  constructor({ fetch = window.fetch }) {
    this._fetch = fetch;
  }
}

// or

function defaultGetIdentityPool() {
  if (typeof window === 'undefined') {
    return;
  }
  return window.fetch('...', ...);
}

function defaultPostEvent() {
  if (typeof window === 'undefined') {
    return;
  }
  return window.fetch('...', ...);
}

class RUM {
  private _getIdentityPool;
  private _postEvent;
  constructor({
    getIdentityPool = defaultGetIdentityPool,
    postEvent = defaultPostEvent,
  }) {
    this._getIdentityPool = getIdentityPool
    this._postEvent = postEvent;
  }
}

// User story:
// As a test runner, I want to mock and monitor RUM.
const TEST_POST_EVENT = jest.fn();
new RUM({
  postEvent: TEST_POST_EVENT,
});

describe('my application', () => {
  it('should emit web vitals', () => {
    myApplication();
    expect(TEST_POST_EVENT).toHaveBeenCalledWith(...); // <-- web vitals
  });

  it('should emit my.custom.event', () => {
    myApplication();
    someUserAction();
    expect(TEST_POST_EVENT).toHaveBeenCalledWith(...); // <-- my.custom.event
  });
});

Again, that's creep-scope, but I think it would help testability. No-op is a great starting point that's compatible with the above stretch goal.

Thanks for the attention to the issue. 🙂

moltar commented 9 months ago

Agree with @CharlesStover here. Would be great to have a mock/stub implementation of the RUM library.

Not only for SSR purposes but also for testability.

A great example of this mocking ability is Undici:

https://undici.nodejs.org/#/docs/api/MockClient

This greatly simplifies testing, and don't need to monkey patch or use jest.mock.

quisido commented 9 months ago

@moltar If you are using React, my package aws-rum-react comes with a MockAwsRumProvider that you can wrap your test in to mock implementation of the RUM library. It still doesn't support SSR per se, but it can help with unit tests. :)

aws-observability / aws-rum-web