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

feat: errorFormatter when using appRouter.createCaller #4120

Closed
1 task done
Patrick-Ullrich opened this issue Mar 31, 2023 · 6 comments
Closed
1 task done

feat: errorFormatter when using appRouter.createCaller #4120

Patrick-Ullrich opened this issue Mar 31, 2023 · 6 comments

Comments

@Patrick-Ullrich
Copy link

Patrick-Ullrich commented Mar 31, 2023

Provide environment information

  System:
    OS: macOS 13.2.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 3.36 GB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.14.0 - ~/.nvm/versions/node/v16.14.0/bin/node
    Yarn: 1.22.18 - ~/.nvm/versions/node/v16.14.0/bin/yarn
    npm: 8.14.0 - ~/.nvm/versions/node/v16.14.0/bin/npm
  Browsers:
    Chrome: 111.0.5563.146
    Firefox: 111.0.1
    Safari: 16.3

"typescript": "4.9.5"
"@trpc/server": "^10.13.0",
"@trpc/client": "^10.13.0",
"@trpc/next": "^10.13.0",
"@trpc/react-query": "^10.13.0",

using @trpc/server 10.0.0 in reproduction

Describe the bug

When using trpc in the actual app with a middleware (e.g. Express middleware):

 trpcExpress.createExpressMiddleware({
                router: appRouter,
                createContext: createTRPCContextFromExpress,
            }),

The error formatter is being called as expected. In my tests where I simply call the router directly using createCaller, the error formatter is not being triggered.

Link to reproduction

https://stackblitz.com/edit/trpc-server-inferred-type-cannot-be-named-x2pseq?file=src/index.ts

To reproduce

See stackblitz

Additional information

For context, we try to keep trpc errors out of our application logic, so we use the errorFormatter to turn our own exceptions into trpc ones:

e.g.:

export const t = initTRPC
    .context<typeof createInnerTRPCContext>()
    .meta<Meta>()
    .create({
        errorFormatter({ error, shape }) {
            if (error.cause instanceof ValidationException) {
                return {
                    ...shape,
                    data: {
                        ...shape.data,
                        code: 'BAD_REQUEST',
                        httpStatus: 400,
                        errors: error.cause.errors,
                    },
                };
          ...

            let eventId = '';
            // Sentry
            if (process.env.NODE_ENV === 'production') {
                eventId = Sentry.captureException(error);
            }

            return {
                ...shape,
                data: {
                    ...shape.data,
                    code: 'INTERNAL_SERVER_ERROR',
                    httpStatus: 500,
                    message: "We're sorry, something went wrong.",
                    referenceId: eventId,
                },
            };
        },
    });

Without it being triggered we are actually just getting TRPCError in the tests which isn't that useful.

👨‍👧‍👦 Contributing

  • 🙋‍♂️ Yes, I'd be down to file a PR fixing this bug!
@KATT
Copy link
Member

KATT commented Apr 2, 2023

This works as designed and would be a breaking change - the error formatting is specific for how responses are serialized before returning it in an API request.

That said, I understand the confusion and we could do some caller that does the complete serialization too.

@KATT KATT changed the title bug: errorFormatter ignored when using appRouter.createCaller feat: errorFormatter when using appRouter.createCaller Apr 2, 2023
@Nick-Lucas
Copy link
Contributor

This works as designed and would be a breaking change - the error formatting is specific for how responses are serialized before returning it in an API request.

That said, I understand the confusion and we could do some caller that does the complete serialization too.

So having fielded the original query on discord, my sense is that anything we pass to initTRPC should work in every situation, including createCaller. Where we don't think that's appropriate it should be in the Adapter layer instead.

Might be one for #3496 or a future v12

@Patrick-Ullrich
Copy link
Author

Patrick-Ullrich commented Apr 3, 2023

I figured this might be the reason. Personally, I looked at error formatter as part of the middleware which probably should be run when using the caller. But I can also sympathize for the choice of not doing so.

It be nice to be able to get it to run somehow though. Thoughts on introducing a... "TestAdapter" similar to Express / NextJS adapter that is a thin wrapper that would trigger the error formatter (and anything else it might need?). That way it wouldn't be a breaking change :)

If you could point me into the right direction (assuming taking a look at one of the adapters) or have some thoughts around it I could take a crack at it. - I am also now on the discord, happy to chat more there as well

@KATT
Copy link
Member

KATT commented Apr 16, 2023

My experience of doing my own integration testing is that you want all information possible even when things fails, including the original error etc. With this, you lose that.

I don't think we'll implement this. For now, I'd recommend you spin up a tRPC server for your tests and create a TRPCClient that you use if you want this behavior. This is how we do most of the testing within trpc itself to assure that our client packages work.

I understand that it is a bit confusing that we have the error formatter on the root object though considering it's only used in adapters.... API design is hard 😅

@KATT KATT closed this as not planned Won't fix, can't repro, duplicate, stale Apr 16, 2023
@KATT
Copy link
Member

KATT commented Apr 16, 2023

If this is blocking you, feel free to reopen or create another feature request on how we can change things to be more clear

@Patrick-Ullrich
Copy link
Author

I appreciate the explanation - yeah, I think i'd still like an easier way to test that specific boundary without having to introduce a web host, but I totally get where you are coming from and I am sure there are more important features to work on.

If I wanted to create my own little adapter that does nothing but run the error formatter on top of it but could still be called using the create Caller... Do you think that be an easy task? If so, could you point me into the right direction? Issue can stayed closed though :)

Thanks again

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants