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

Removes unused try {} catch {} #300

Merged
merged 2 commits into from
Aug 1, 2024

Conversation

simonbs
Copy link
Contributor

@simonbs simonbs commented Jul 31, 2024

This PR removes a try {} catch {} on the sign in page. The catch block merely printed the error. However, this resulted in the following error being logged upon every sign in with GitHub.

Error: NEXT_REDIRECT
    at getRedirectError (webpack-internal:///(rsc)/./node_modules/next/dist/client/components/redirect.js:49:19)
    at redirect (webpack-internal:///(rsc)/./node_modules/next/dist/client/components/redirect.js:60:11)
    at signIn (webpack-internal:///(rsc)/./node_modules/next-auth/lib/actions.js:59:76)
    at async $$ACTION_0 (webpack-internal:///(rsc)/./src/app/auth/signin/page.tsx:252:9)
    at async /Users/simonbs/Developer/shape-docs/node_modules/next/dist/compiled/next-server/app-page.runtime.dev.js:39:418
    at async rw (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/compiled/next-server/app-page.runtime.dev.js:38:7978)
    at async r6 (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/compiled/next-server/app-page.runtime.dev.js:41:1256)
    at async doRender (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/base-server.js:1438:30)
    at async cacheEntry.responseCache.get.routeKind (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/base-server.js:1599:28)
    at async DevServer.renderToResponseWithComponentsImpl (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/base-server.js:1507:28)
    at async DevServer.renderPageComponent (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/base-server.js:1931:24)
    at async DevServer.renderToResponseImpl (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/base-server.js:1969:32)
    at async DevServer.pipeImpl (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/base-server.js:920:25)
    at async NextNodeServer.handleCatchallRenderRequest (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/next-server.js:272:17)
    at async DevServer.handleRequestImpl (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/base-server.js:816:17)
    at async /Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/dev/next-dev-server.js:339:20
    at async Span.traceAsyncFn (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/trace/trace.js:154:20)
    at async DevServer.handleRequest (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/dev/next-dev-server.js:336:24)
    at async invokeRender (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/lib/router-server.js:174:21)
    at async handleRequest (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/lib/router-server.js:353:24)
    at async requestHandlerImpl (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/lib/router-server.js:377:13)
    at async Server.requestListener (/Users/simonbs/Developer/shape-docs/node_modules/next/dist/server/lib/start-server.js:141:13) {
  digest: 'NEXT_REDIRECT;replace;https://github.com/login/oauth/authorize?scope=repo&response_type=code&client_id=...;',
  mutableCookies: p {
    _parsed: Map(3) {
      'authjs.pkce.code_verifier' => [Object],
      'authjs.callback-url' => [Object],
      'authjs.csrf-token' => [Object]
    },
    _headers: _HeadersList {
      cookies: [Array],
      [Symbol(headers map)]: [Map],
      [Symbol(headers map sorted)]: null
    }

As far as I can tell from this blog post this error isn't a "real" error that's meant to be handled but rather a signal to Next.js that it needs to do a redirect.

Since we're not doing any interesting error handling, I thought we might as well remove the try/catch entirely and rely on Next.js to handle any error thrown. At least this avoids polluting our logs with errors that we cannot and should not do anything about.

@simonbs simonbs requested a review from ulrikandersen as a code owner July 31, 2024 07:55
@simonbs simonbs enabled auto-merge July 31, 2024 07:56
Copy link
Contributor

@ulrikandersen ulrikandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense 👍

@simonbs simonbs merged commit 0ad14a0 into develop Aug 1, 2024
9 checks passed
@simonbs simonbs deleted the enhancement/remove-unused-error-handling branch August 1, 2024 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants