Refactor/server folder

[JohanHjelsethStorstad]

This is a big refactor so discuss changes here. I especially am wondering about the auth stuff. Should we have two auth directories? and much of the things in src/auth should use things in auth/server/auth. There are probably many parts of server logic that can be made more modular.

[JohanHjelsethStorstad]

I think that the src/auth dir. should be refactored some time next week in a separate PR.

[JohanHjelsethStorstad]

I think this is now ready. I have not done much with Auth. We might want to change the name ActionReturn to something like WrappedSafeReturn at this point

[JohanHjelsethStorstad]

And I have not done anything with /actions/groups

[Paulijuz]

Another thing i though about is that do server functions really need to return an action return? I mean, they will always have to be called from a server action. (I think at least, i see no reason to call them from server side rendered pages as that would mean that they won't be authed.) So we could just throw errors instead. I think this would make the code base a lot cleaner on the server side of things without affecting the frontend part as we won't have to type this

const doSomethingRes = await doSomething()

if (!doSomethingRes.success) {
    return doSomething
}

const data = doSomethingRes.data

and could simply write instead

const data = await doSomething()

since an error could simply be throw deep down somwhere and be catched at the action call.

[Paulijuz]

We could create a custom error class and have the same error information in it as an action return error. Then we could simply have a function to parse this error from an error class to an action return error. Basically mirroring what we do with prisma.

For example inside an action:

try {
 const newThing = await createThing()

 return { success: true, data: newThing }
} catch(e) {
 return createActionError(e)
}

[JohanHjelsethStorstad]

I did think about this initially, but did not do it as I a) would like to have a common way of handling errors and b) I feel we are then sacrificing how clean the actions are going to be. The thing is that I really do not like the way we do it for prisma, ref the error part of #207. An option is obviously to move this try-catch wrapper out to the action leve, that is use the prismaCall function as a generalized "safeTryCatch" that returns a ActionReturn. so we would inside the action say

return await safeTryCatch(() => createThing(data))

But now this trycatch must address all errors both with prisma and other things.

[JohanHjelsethStorstad]

Just wondering: If we are to move part of zod-validation into the server-folder how would you for example then throw a zod-error. I like the return creaeZodActionError(parse) pattern, would you then just throw this object inside the server dir. for it to be caught in the action? - and now how does the try catch block or safeTryCatch block know that the error it has caught is already of type ActionReturn?

[JohanHjelsethStorstad]

We could create a custom error class and have the same error information in it as an action return error. Then we could simply have a function to parse this error from an error class to an action return error. Basically mirroring what we do with prisma.

For example inside an action:

try {
 const newThing = await createThing()

 return { success: true, data: newThing }
} catch(e) {
 return createActionError(e)
}

Didn't see the custom error part. If you do an Instance of check on custom error then I agree. But I still don't like try catch. And then we also scrap prismaCall and just use safeTryCatch that in the chech block matches Error instances with action returns of success false

[JohanHjelsethStorstad]

Or should call to prisma in /server errors be translated to custom errors so actions will do UNKNOWN ERROR on all errors no of custom type. Or should actions handle Prisma-errors itself?

[Paulijuz]

I don't really like the try catch pattern either. Especially not the way JavaScript does it. But i do like being able to throw errors since that simplifies how the error reaches the top.

When it comes to prisma errors maybe they can be left as is since I don't see what value we get if we simply wrap them.

[Paulijuz]

On the other hand by wrapping the prisma error we can tell what the error is for. If we went with the first approach then if you try to call a create action that calls some other create actions and one of them fails you'd only get what type of error it is and not which one of them failed.

[Paulijuz]

My suggestion:

export type ErrorCode =
| 'DUPLICATE'
| 'NOT FOUND'
| 'BAD PARAMETERS'
| 'UNAUTHENTICATED'
| 'UNAUTHORIZED'
| 'UNKNOWN ERROR'

export type ErrorMessage = {
    path?: (number | string)[],
    message: string,
}

export class ServerError extends Error {
    errorCode: ErrorCode;
    errors: ErrorMessage[];

    constructor(errorCode: ErrorCode, errors: string | ErrorMessage[]) {
        const parsed_errors = typeof errors === 'string'
            ? [{ message: errors }]
            : errors

        super(errorCode)

        this.errorCode = errorCode
        this.errors = parsed_errors
        this.name = "ServerError"
    }
}

[Paulijuz]

Then the types for actions errors can be:

import { ErrorCode, ErrorMessage } from "@/server/error"

export type ActionReturnError = {
    success: false,
    errorCode: ErrorCode,
    error?: ErrorMessage[],
}

export type ActionReturn<ReturnType, DataGuarantee extends boolean = true> = (
    ActionReturnError 
) | {
    success: true,
} & (
    DataGuarantee extends true ? {
        data: ReturnType
    } : {
        data?: ReturnType
    }
)

[Paulijuz]

You can check this out in the branch refactor/server-folder-and-error

I haven't refactored the functions to use this, just added the error class.