Skip to content

Fix Redux integration failing with reducer injection #16017

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

Open
3 tasks done
AntoineDuComptoirDesPharmacies opened this issue Apr 9, 2025 · 4 comments · May be fixed by #16106
Open
3 tasks done

Fix Redux integration failing with reducer injection #16017

AntoineDuComptoirDesPharmacies opened this issue Apr 9, 2025 · 4 comments · May be fixed by #16106

Comments

@AntoineDuComptoirDesPharmacies

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/react

SDK Version

9.9.0

Framework Version

18.2.0

Link to Sentry event

No response

Reproduction Example/SDK Setup

import { configureStore } from '@reduxjs/toolkit';
import { combineReducers } from 'redux';
import createSagaMiddleware from 'redux-saga';
import * as Sentry from '@sentry/react';

function createReducer(injectedReducers = {}) {
  const rootReducer = combineReducers({
    global: globalReducer,
    ...injectedReducers,
  });

  return rootReducer;
}


[...]

  const initialState = {};

  const sagaMiddleware = createSagaMiddleware();

  const sentryReduxEnhancer = Sentry.createReduxEnhancer({
    configureScopeWithState: (scope, state) => {
      console.log("configurescope", state);
    },
  });

  const store = configureStore({
    preloadedState: initialState,
    reducer: createReducer(),
    middleware: (getDefaultMiddleware) => {
      return getDefaultMiddleware().concat(sagaMiddleware)
    },
    enhancers: (getDefaultEnhancers) => {
      return getDefaultEnhancers().concat(sentryReduxEnhancer);
    },
  });

[...]

const injectedReducers= {
  newKey : (state, action) => {
       // simplification just for the reproducer
       return state;
   }
}

store.replaceReducer(createReducer(injectedReducers));

Steps to Reproduce

  1. Code an app using code splitting for reducer injection (See : https://redux.js.org/usage/code-splitting#reducer-injection-approaches).
    Reducers are added on runtime depending of the section of the app loaded by user.
  2. Load the app and go to the lazy loaded reducer section
  3. Create a sentry issue that will upload redux state and check that your event redux state is missing keys/values modified after the reducer injection

Expected Result

Sentry Redux Enhancer should not be "evicted" when calling replaceReducer method and so configureScopeWithState should continue to be called on state change.

See this issue about how to code a redux enhancer that will remain even if replaceReducer is called :
reduxjs/redux#4127

Actual Result

Function 'replaceReducer' fully remove the Sentry Redux Enhancer from the chain.
As a result, the method configureScopeWithState is no longer called on redux state change.

Same problem occured here and likely to be the came cause :

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Apr 9, 2025
@github-actions github-actions bot added the Package: react Issues related to the Sentry React SDK label Apr 9, 2025
@lforst
Copy link
Member

lforst commented Apr 9, 2025

Only looking superficially at this, my main thought is whether there is anything that the Sentry SDK can even do here. Isn't this more an issue with redux toolkit and/or your setup?

@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Apr 9, 2025
@getsantry getsantry bot moved this to Waiting for: Community in GitHub Issues with 👀 3 Apr 9, 2025
AntoineDuComptoirDesPharmacies added a commit to LeComptoirDesPharmacies/sentry-javascript that referenced this issue Apr 9, 2025
Override replaceReducer functiont so any lazy loaded injection will reinsert the sentry reducer in the chain
@AntoineDuComptoirDesPharmacies
Copy link
Author

Hi @lforst,

According to timdorr and markerikson :

That isn't an enhancer. It's just a wrapper function on createStore

According to Methuselah96, is it :

a bug in your enhancer. If your enhancer messes with the reducer, it needs to re-implement the replaceReducer function

Here is a commit that follow their principles, by overriding the replaceReducer method.
develop...LeComptoirDesPharmacies:sentry-javascript:bugfix/16017

I have to admit I am not perfect in TS and not have a build environmlent right now to test it, but I think it may be the solution.

@getsantry getsantry bot moved this from Waiting for: Community to Waiting for: Product Owner in GitHub Issues with 👀 3 Apr 9, 2025
@mydea mydea added the Bug label Apr 11, 2025 — with Linear
@s1gr1d
Copy link
Member

s1gr1d commented Apr 11, 2025

An enhancer takes a store creator function as input (next) and returns a new, enhanced store creator function.

The current Sentry Redux enhancer correctly follows this pattern (so it's a legit enhancer):

return (next: StoreEnhancerStoreCreator): StoreEnhancerStoreCreator =>
  <S = any, A extends Action = AnyAction>(reducer: Reducer<S, A>, initialState?: PreloadedState<S>) => {
    // Enhancement logic
    
    return next(sentryReducer, initialState);
  };

However, you are correct that replaceReducer is missing in the current code as the Sentry enhancer is modifying the reducer.

@AntoineDuComptoirDesPharmacies Do you want to contribute this and create a PR for this? If you do, please assign me as reviewer so I get a notification about it.

@getsantry getsantry bot moved this from Waiting for: Product Owner to Waiting for: Community in GitHub Issues with 👀 3 Apr 11, 2025
@mydea mydea changed the title Redux toolkit integration fail with reducer injection Fix Redux integration failing with reducer injection Apr 14, 2025
@stephanie-anderson stephanie-anderson removed the Package: react Issues related to the Sentry React SDK label Apr 16, 2025
AntoineDuComptoirDesPharmacies added a commit to LeComptoirDesPharmacies/sentry-javascript that referenced this issue Apr 22, 2025
Add a test to verify that redux enhancer is still calling setContext with updated store avec reducer being replaced
@AntoineDuComptoirDesPharmacies
Copy link
Author

Thanks for the response and proposal @s1gr1d. It seems clear.
I just created a PR with an additional test to ensure future non regression.
Thanks @Lms24 for adding @s1gr1d as reviewer. 👍

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Apr 22, 2025
AntoineDuComptoirDesPharmacies added a commit to LeComptoirDesPharmacies/sentry-javascript that referenced this issue Apr 22, 2025
To keep correct typescript type, use "Proxy" instead of plain object.
@s1gr1d I am wondering if we can do better than accessing the index [0] directly ?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
5 participants