Unexpected behaviour when redirecting in action
liegeandlief opened this issue · 23 comments
What version of Remix are you using?
1.4.1
Steps to Reproduce
Create a Remix project (with the built in Remix server) with the following routes:
index.tsx:
import { memo } from 'react'
const IndexRoute = memo(() => {
return <h1>Home</h1>
})
IndexRoute.displayName = 'IndexRoute'
export default IndexRoutemagic-link.tsx:
import { memo, useEffect } from 'react'
import { LoaderFunction, json, useLoaderData, useSubmit } from 'remix'
import { emailFormFieldName, codeFormFieldName } from '~/routes/verify-magic-link'
type LoaderData = {
email: string,
code: string
}
const loader: LoaderFunction = async ({ request }) => {
const url = new URL(request.url)
const email = url.searchParams.get("email") ?? ''
const code = url.searchParams.get("code") ?? ''
return json<LoaderData>({
email,
code
})
}
const MagicLinkRoute = memo(() => {
const loaderData = useLoaderData<LoaderData>()
const submit = useSubmit()
useEffect(() => {
const formData = new FormData()
formData.append(emailFormFieldName, loaderData.email)
formData.append(codeFormFieldName, loaderData.code)
submit(formData, { method: 'post', action: '/verify-magic-link' })
}, [loaderData.email, loaderData.code, submit])
return (
<p>Some pending UI</p>
)
})
MagicLinkRoute.displayName = 'MagicLinkRoute'
export default MagicLinkRoute
export { loader }verify-magic-link.tsx:
import { memo } from 'react'
import { ActionFunction, json, redirect, useActionData } from 'remix'
export const emailFormFieldName = 'email'
export const codeFormFieldName = 'code'
// fake verifyMagicLink function with forced failure
const verifyMagicLink = ({ email, code } : { email: string, code: string }) => Promise.resolve(false)
type ActionData = Awaited<ReturnType<typeof verifyMagicLink>>
const action: ActionFunction = async ({ request }) => {
const body = await request.formData()
const email = body.get(emailFormFieldName)?.toString() ?? ''
const code = body.get(codeFormFieldName)?.toString() ?? ''
const verifyMagicLinkResult = await verifyMagicLink({ email, code })
if (verifyMagicLinkResult === true) {
return redirect('/')
}
return json<ActionData>(verifyMagicLinkResult)
}
const VerifyMagicLinkRoute = memo(() => {
const actionData = useActionData<ActionData>()
return (
<p>{JSON.stringify(actionData, null, 2)}</p>
)
})
VerifyMagicLinkRoute.displayName = 'VerifyMagicLinkRoute'
export default VerifyMagicLinkRoute
export { action }In the entry.server.tsx handleRequest function put the following just before returning the response:
console.log({
type: 'handleRequest',
url: request.url,
method: request.method,
response: newResponse.status
})In the entry.server.tsx handleDataRequest function put the following just before returning the response:
console.log({
type: 'handleDataRequest',
url: request.url,
method: request.method,
response: newResponse.status
})Go to https://localhost:3000/magic-link?code=123456&email=test@test.com in your browser. In the terminal you should see:
{
type: 'handleRequest',
url: 'http://localhost:3000/magic-link?code=123456&email=test@test.com',
method: 'GET',
response: 200
}
{
type: 'handleDataRequest',
url: 'http://localhost:3000/verify-magic-link?_data=routes%2Fverify-magic-link',
method: 'POST',
response: 200
}
{
type: 'handleDataRequest',
url: 'http://localhost:3000/verify-magic-link?_data=root',
method: 'GET',
response: 200
}This is what I would expect to see.
Now in verify-magic-link.tsx change const verifyMagicLink = ({ email, code } : { email: string, code: string }) => Promise.resolve(false) to const verifyMagicLink = ({ email, code } : { email: string, code: string }) => Promise.resolve(true).
Run the same request in your browser again.
Expected Behavior
Terminal should show:
{
type: 'handleRequest',
url: 'http://localhost:3000/magic-link?code=123456&email=test@test.com',
method: 'GET',
response: 200
}
{
type: 'handleDataRequest',
url: 'http://localhost:3000/verify-magic-link?_data=routes%2Fverify-magic-link',
method: 'POST',
response: 302
}
{
type: 'handleDataRequest',
url: 'http://localhost:3000/?_data=root',
method: 'GET',
response: 200
}Actual Behavior
Terminal shows:
{
type: 'handleRequest',
url: 'http://localhost:3000/magic-link?code=123456&email=test@test.com',
method: 'GET',
response: 200
}
{
type: 'handleDataRequest',
url: 'http://localhost:3000/?_data=root',
method: 'GET',
response: 200
}The POST request does not get logged by either handleRequest or handleDataRequest.
Even though I am taken to the index.tsx route as expected the 302 redirect response never seems to be returned to the browser. Instead when I look in the browser network tab I can see that the the response to the POST request was a 204 response.
This is causing me issues because I would like to add a set-cookie header on the redirect response but the cookie never gets set because the redirect response doesn't make it to the browser.
When a loader/action returns a redirect from a client-side navigation, Remix uses status 204 and a header X-Remix-Redirect. This is so that the actual fetch call doesn't try to redirect, but instead Remix will handle the redirect. Also, if the response includes a set-cookie header, it will add X-Remix-Revalidate to ensure that all loaders are revalidated.
remix/packages/remix-server-runtime/server.ts
Lines 135 to 149 in a2e1449
Okay that makes sense but in that case it seems to be bypassing handleDataRequest when returning the 204 response. In handleDataRequest I make some changes to the response by adding extra headers and I need these headers to be returned. But these headers don't get added to the 204 response.
If Remix needs to amend the response status should it not do it after handleDataRequest has completed?
handleRequest and handleDataRequest are the last things that happen before returning to the client. Remix has already done its processing by then. These are not pre request handlers... but post Remix handlers.
But the point is that handleDataRequest is not being run in this scenario and Remix is returning to the client before running it. Should it not always run for every data request? It seems strange to have a way to hook into the request lifecycle which only runs for some requests. Without an understanding of the internals of Remix it is hard to understand when it will or won't be executed.
Would this not be a simple change in packages/remix-server-runtime/server.ts from:
return new Response(null, {
status: 204,
headers,
});
to:
response = new Response(null, {
status: 204,
headers,
});
I just ran into this issue but on Microsoft Edge only; everything worked fine on Chrome and Firefox. I am redirecting on an Action submission (passing OAuth info to the backend) and I can see both the X-Remix-Redirect and X-Remix-Revalidate headers (as well as my session being set) on the response. I ended up having to redirect on the client side (as well?) to make it work consistently:
const submiter = useFetcher();
const [searchParams] = useSearchParams();
const redirectTo = searchParams.get('redirectTo') ?? '/actions';
const navigate = useNavigate();
useEffect(() => {
if (!isBrowser || submiter.type !== 'init') return;
const formData = new FormData();
const contentHash = window.location.hash;
if (hashContainsKnownProperties(contentHash)) {
const hash = getDeserializedHash(contentHash);
if (hash.error) {
formData.append('error', hash.error);
} else {
Object.entries(hash).forEach(([key, value]) =>
formData.append(key, value as string),
);
}
window.location.hash = '';
}
formData.append('redirect_to', redirectTo);
submiter.submit(formData, {
action: '/oauth/callback',
method: 'post',
});
}, [submiter, redirectTo]);
useEffect(() => {
if (submiter.type === 'done') {
navigate(redirectTo, { replace: true });
}
}, [navigate, redirectTo, submiter.type]);
return null;I seem to have the same issue with the remix-auth library, the redirect to the Github OAuth flow seems to not happen properly and I have a 204. It used to work with remix 1.4.3
Update:
It actually still works in 1.6.4, but starts breaking in 1.6.5
Update 2:
I think I managed to pinpoint my own issue, I don't know if this is related to the original issue of this thread, but remix-auth-github properly redirects when <LiveReload /> is commented. The only change I see from 1.6.5 is this PR: #859
@fknop I had the same issue - lots of weird things happening with LiveReload and remix-auth, especially re: redirecting when trying to log in. I verified that using the old version of LiveReload stops this problem from happening. Here's the "custom" LiveReload function I'm using instead of the production one from remix (which is just the 1.6.4 version with no other changes):
export const LiveReload =
process.env.NODE_ENV !== 'development'
? () => null
: function LiveReload({
port = Number(process.env.REMIX_DEV_SERVER_WS_PORT || 8002),
nonce = undefined,
}) {
let js = String.raw;
return (
<script
nonce={nonce}
suppressHydrationWarning
dangerouslySetInnerHTML={{
__html: js`
(() => {
let protocol = location.protocol === "https:" ? "wss:" : "ws:";
let host = location.hostname;
let socketPath = protocol + "//" + host + ":" + ${String(
port,
)} + "/socket";
let ws = new WebSocket(socketPath);
ws.onmessage = (message) => {
let event = JSON.parse(message.data);
if (event.type === "LOG") {
console.log(event.message);
}
if (event.type === "RELOAD") {
console.log("💿 Reloading window ...");
window.location.reload();
}
};
ws.onerror = (error) => {
console.log("Remix dev asset server web socket error:");
console.error(error);
};
})();
`,
}}
/>
);
};The old version of LiveReload solves my issues as well. I believe it breaks due to this added block:
ws.onclose = (error) => {
console.log("Remix dev asset server web socket closed. Reconnecting...");
setTimeout(
() =>
remixLiveReloadConnect({
onOpen: () => window.location.reload(),
}),
1000
);
};More specifically, I think the arbitrary 1000 ms timeout before reloading creates a race condition, where, during a redirect, the route you're navigating from will reload itself before the redirect has been able to complete. By increasing the timeout, the issues disappeared for my part, but that does not seem like a viable permanent fix.
Hello, I have tested this behaviour with last remix version as of now (1.7.2) and it is still happening.
I am running a 2020 Macbook Air with M1 Proccesor (big sur, 11.6 os version( and, weirdly, this bug happens only on Firefox (version 104.0.2). On Chrome and Safari, redirects work ok.
I tried replacing the LiveReload component with the one suggested in this thread and it worked, problem was solved. With this I can be 100% sure that the bug is caused by the setTimeout on the LiveReload code
I confirmed that <LiveReload /> is causing problems when dev with redirecting in Firefox (e.g. using remix-auth). It doesn't seem to cause any problems in Chrome or Safari.
Thank you @ErickTamayo I've spent like 2 hours going nuts why my redirect is not working, I can also confirm that it only happens in Firefox and if I remove <LiveReload /> everything works just fine.
As mentioned before this was introduced in #859 to enable automatic re-connection when the dev server is restarted. Would it be possible to check for the close event code before setting the timeout? This would keep the auto reconnect behavior while fixing the Firefox behavior (at least in a quick test):
- ws.onclose = (error) => {
+ ws.onclose = (event) => {
console.log("Remix dev asset server web socket closed. Reconnecting...");
- setTimeout(
+ if (event.code === 1006) setTimeout(
() =>
remixLiveReloadConnect({
onOpen: () => window.location.reload(),
}),
1000
);
};LiveReload component code
export const LiveReload =
process.env.NODE_ENV !== "development"
? () => null
: function LiveReload({
port = Number(process.env.REMIX_DEV_SERVER_WS_PORT || 8002),
nonce = undefined,
}: {
port?: number;
/**
* @deprecated this property is no longer relevant.
*/
nonce?: string;
}) {
let js = String.raw;
return (
<script
nonce={nonce}
suppressHydrationWarning
dangerouslySetInnerHTML={{
__html: js`
function remixLiveReloadConnect(config) {
let protocol = location.protocol === "https:" ? "wss:" : "ws:";
let host = location.hostname;
let socketPath = protocol + "//" + host + ":" + ${String(
port
)} + "/socket";
let ws = new WebSocket(socketPath);
ws.onmessage = (message) => {
let event = JSON.parse(message.data);
if (event.type === "LOG") {
console.log(event.message);
}
if (event.type === "RELOAD") {
console.log("💿 Reloading window ...");
window.location.reload();
}
};
ws.onopen = () => {
if (config && typeof config.onOpen === "function") {
config.onOpen();
}
};
ws.onclose = (event) => {
console.log("Remix dev asset server web socket closed. Reconnecting...");
if (event.code === 1006) setTimeout(
() =>
remixLiveReloadConnect({
onOpen: () => window.location.reload(),
}),
1000
);
};
ws.onerror = (error) => {
console.log("Remix dev asset server web socket error:");
console.error(error);
};
}
remixLiveReloadConnect();
`,
}}
/>
);
};I've also done a quick test with @danieljb's event.code fix above and it appears to fix the issue in Firefox.
Thanks guys! I had a headache for 2 days straight because I couldn't find the issue... Any PRs happening for this?
I believe PR 4725 fixes this issue.
I'm testing a redirection while authenticating users and the POST request is not triggering the action so the redirection is not happening until the user refresh the page. I'm using RemixJS 1.11.0 and React 18.2.0. Please check the current implementation:
const csrf = useAuthenticityToken();
const { web3AuthState } = useWeb3AuthContext();
const submit = useSubmit();
useEffect(() => {
if (web3AuthState.state === 'connected') {
console.log('Logged in with Web3Auth')
submit(
{ ...web3AuthState, csrf },
{ replace: true, method: 'post' }
);
}
}, [web3AuthState]);I added logs from the action of this route but they are not called at any time from Firefox 🤔
Any help is really appreciated!
While the useSubmit hook is fixed, I was able to solve this wrong behavior by using a hidden form element directly:
const formRef = useRef<HTMLFormElement>(null);
useEffect(() => {
if (web3AuthState.state === 'connected' && formRef.current) {
console.log('Logged in with Web3Auth')
formRef.current?.submit(); // Fixed by replacing useSubmit in order to trigger the submit of the auth redirection
}
}, [web3AuthState, formRef]);Let me know what you think folks! <3
Hum, I feel like a lot of issues have been mixed into this one.
@liegeandlief, is this issue still relevant for you?
This issue has been automatically closed because we haven't received a response from the original author 🙈. This automation helps keep the issue tracker clean from issues that are unactionable. Please reach out if you have more information for us! 🙂
@machour can you reopen this issue please?
During my tests with useSubmit, I'm getting a 201 status code instead of a 302 redirection. can you validate that wrong behavior?
This is the code of that action:
export const action = async ({ request }: ActionArgs) => {
try {
// validations here, redirecting users with this action submit :)
return redirect('/profile',
{
status: 302,
headers: {
'Set-Cookie': await saveLoginCookie(),
}
}
);
} catch (error) {
console.error('Auth error', error);
return json(`Sorry, we couldn't authenticate the user`, {
status: 500,
});
}
};I am new to Remix. Just started last week.
I am using latest indie stack.
After I build the app for production, and run it in production mode using
cross-env NODE_ENV=production remix-serve build
Safari: After login it will not redirect to the Notes page.
Chrome: After login redirecting to Notes page just fine.
My observation: Safari seems to be not redirecting due to secure: true turned on in the createCookieSessionStorage setting.
When I forced secure to false it works fine. That explains why this is working okay in npm run dev and not npm start
export const sessionStorage = createCookieSessionStorage({
cookie: {
name: "__session",
httpOnly: true,
path: "/",
sameSite: "lax",
secrets: [process.env.SESSION_SECRET],
secure: process.env.NODE_ENV === "test", // won't work when its set to production
},
});Let me know what I can do to improve?
Hey folks! As @machour noted this issue seems to now have split into at least 3-4 different potential issue discussions so it's near impossible to continue triaging. I'm going to close this out and ask the following:
- If you still have an issue on the current release, please open a new issue with a minimal reproduction via https://remix.new
- Please comment with any newly opened issues so folks can check before opening duplicates of one another
Thank you! This will make it much easier for us to debug any potential issues.