react-admin icon indicating copy to clipboard operation
react-admin copied to clipboard

Reject promise in checkError with logoutUser: false and without redirectTo causes an error

Open thibault-barrat opened this issue 1 year ago • 4 comments

What you were expecting: I would like to notify user when getPermissions request is failing. To do it, I catch the error in checkError and I reject promise with logoutUser: false and without redirectTo as I just want to display a notification without logout or redirect the user

What happened instead: The notification is correctly displayed but I have a JS error:

TypeError: can't access property "startsWith", redirectTo is undefined

This comes from useLogoutIfAccessDenied : https://github.com/marmelab/react-admin/blob/75a6683c9a64d6190ddde88d043b8e15b48c39e6/packages/ra-core/src/auth/useLogoutIfAccessDenied.ts#L103-L113

Steps to reproduce: Update getPermissions and checkError with following code:

getPermissions: () => {
	return Promise.reject('getPermissions error');
},
checkError: (error) => {
	if (error.status === 401 || error.status === 403) {
		return Promise.reject({ message: 'Unauthorized' });
	}
	if (error === 'getPermissions error') {
		return Promise.reject({
			message: error,
			logoutUser: false,
		});
	}
	return Promise.resolve();
},

Environment

  • React-admin version: 4.16.17
  • React version: 18.2.0
  • Stack trace (in case of a JS error):
    logoutIfAccessDenied useLogoutIfAccessDenied.ts:105
    step chunk-6GJJR47V.js:4115
    verb chunk-6GJJR47V.js:4062
    __awaiter2 chunk-6GJJR47V.js:4048
    __awaiter2 chunk-6GJJR47V.js:4030
    logoutIfAccessDenied useLogoutIfAccessDenied.ts:51
    promise callback*useLogoutIfAccessDenied/logoutIfAccessDenied< useLogoutIfAccessDenied.ts:51
    onError usePermissions.ts:57
    NotifyManager notifyManager.js:62
    notifyFn notifyManager.js:10
    flush notifyManager.js:77
    flush notifyManager.js:76
    batchedUpdates$1 React
    flush notifyManager.js:75
    promise callback*scheduleMicrotask utils.js:322
    flush notifyManager.js:74
    batch notifyManager.js:30
    dispatch query.js:392
    onError query.js:348
    reject2 retryer.js:67
    run2 retryer.js:132
    promise callback*run2 retryer.js:116
    run2 retryer.js:149
    promise callback*Retryer2/run2/< retryer.js:145
    promise callback*run2 retryer.js:116
    run2 retryer.js:149
    promise callback*Retryer2/run2/< retryer.js:145
    promise callback*run2 retryer.js:116
    run2 retryer.js:149
    promise callback*Retryer2/run2/< retryer.js:145
    promise callback*run2 retryer.js:116
    Retryer2 retryer.js:156
    fetch query.js:332
    executeFetch queryObserver.js:199
    onSubscribe queryObserver.js:40
    subscribe subscribable.js:16
    useBaseQuery useBaseQuery.js:60
    React 13
    workLoop scheduler.development.js:266
    flushWork scheduler.development.js:239
    performWorkUntilDeadline scheduler.development.js:533
    js scheduler.development.js:571
    js scheduler.development.js:633
    __require2 chunk-GFT2G5UO.js:18
    js index.js:6
    __require2 chunk-GFT2G5UO.js:18
    React 2
    __require2 chunk-GFT2G5UO.js:18
    js React
    __require2 chunk-GFT2G5UO.js:18
    js React
    __require2 chunk-GFT2G5UO.js:18
    <anonymous>

thibault-barrat avatar Aug 30 '24 08:08 thibault-barrat

Thanks for this report!

Providing no redirectTo is currently not supported, but it would be nice if it was!

I'll label this issue as enhancement (and good first issue). Thanks!

slax57 avatar Aug 30 '24 08:08 slax57

I had fixed it in https://github.com/marmelab/react-admin/pull/10177. Could you please assign it to me ?

rktamil avatar Sep 01 '24 00:09 rktamil

@rktamil No need to assign the issue. Referencing the issue number in your PR description is enough. Thank you for contributing!

slax57 avatar Oct 07 '24 16:10 slax57

I have created a PR which solves this

carloshv93 avatar May 30 '25 20:05 carloshv93

Fixed by https://github.com/marmelab/react-admin/pull/10763

slax57 avatar Jun 19 '25 08:06 slax57