Skip to content
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

redirect to external site in root.tsx loader doesn't work, still calls App #256

Closed
cjkooij opened this issue Jul 3, 2023 · 13 comments
Closed
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@cjkooij
Copy link
Contributor

cjkooij commented Jul 3, 2023

Trying to return a redirect from the root.tsx loader doesn't redirect and tries to render the App.

See screenshots below, redirect is the only line added on a freshly installed epic-stack.

image image
@cjkooij
Copy link
Contributor Author

cjkooij commented Jul 3, 2023

throwing redirect does work, credit to @jacobparis

@kentcdodds
Copy link
Member

I'm curious why returning a redirect doesn't work though. Perhaps this is a bug for Remix?

@cjkooij
Copy link
Contributor Author

cjkooij commented Jul 3, 2023 via email

@kentcdodds
Copy link
Member

Thanks for checking on that! We'll definitely want to figure out what it is then.

@kentcdodds kentcdodds added bug Something isn't working help wanted Extra attention is needed labels Jul 3, 2023
@onemen
Copy link
Contributor

onemen commented Jul 4, 2023

maybe the bug is in server/index.js

when i replace server/index.js file with the content of remix express/server

with just one change

-  const BUILD_PATH = './build/index.js'
+  const BUILD_PATH = '../build/index.js'

the redirect work as expected

@onemen
Copy link
Contributor

onemen commented Jul 4, 2023

something wrong with sentry wrapExpressCreateRequestHandler, when removed from server/index.js redirect work as expected

import path from 'path'
import { fileURLToPath } from 'url'
import express from 'express'
import chokidar from 'chokidar'
import compression from 'compression'
import morgan from 'morgan'
import address from 'address'
import closeWithGrace from 'close-with-grace'
import helmet from 'helmet'
import crypto from 'crypto'
-import {
-  type RequestHandler,
-  createRequestHandler as _createRequestHandler,
-} from '@remix-run/express'
+import { type RequestHandler, createRequestHandler } from '@remix-run/express'
import { wrapExpressCreateRequestHandler } from '@sentry/remix'
import { type ServerBuild, broadcastDevReady } from '@remix-run/node'
import getPort, { portNumbers } from 'get-port'
import chalk from 'chalk'

// @ts-ignore - this file may not exist if you haven't built yet, but it will
// definitely exist by the time the dev or prod server actually runs.
import * as remixBuild from '../build/index.js'
const MODE = process.env.NODE_ENV

-const createRequestHandler = wrapExpressCreateRequestHandler(
-  _createRequestHandler,
-)

const BUILD_PATH = '../build/index.js'

@kentcdodds
Copy link
Member

Hey @scefali, do you have any idea what may be causing this issue? Looks like it's only present when Sentry's wrapper is involved.

@scefali
Copy link
Contributor

scefali commented Jul 5, 2023

@kentcdodds I do not but I'm having someone take a look

@AbhiPrasad
Copy link

@onurtemizkan something lets take a look at?

This might be fixed by us merging and releasing getsentry/sentry-javascript#8415

Sorry for the trouble folks!

@AbhiPrasad
Copy link

Also if you could run npm why @sentry/remix and report the version would help us debug a lot!

@AbhiPrasad
Copy link

We'll try and get a fix out friday or start of next week, thanks for your patience everyone.

@AbhiPrasad
Copy link

We've released a new version of @sentry/remix with the fix in 7.58.0. Can you try updating your SDK version and seeing if the error persists? Thanks!

@kentcdodds
Copy link
Member

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants