Skip to content

(thunderstore-api) Implement user-facing error handling improvements and update tests for authentication errors #1614

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: 11-19-implement_nimbus_error_handling_components_and_utilities_for_consistent_user-facing_error_messages
Choose a base branch
from
Open

(thunderstore-api) Implement user-facing error handling improvements and update tests for authentication errors #1614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e3049e8
(thunderstore-api) Implement user-facing error handling improvements …
Oksamies Dec 2, 2025
21ab014
(thunderstore-api) Refactor API fetch functions to remove async/await…
Oksamies Dec 2, 2025
13fecd4
Correct apiFetch calls in currentUser and dynamicHTML functions
Oksamies Dec 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 31 additions & 23 deletions packages/thunderstore-api/src/apiFetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,36 +44,41 @@ function sleep(delay: number) {
return new Promise((resolve) => setTimeout(resolve, delay));
}

export type apiFetchArgs<B, QP> = {
type SchemaOrUndefined<Schema extends z.ZodSchema | undefined> =
Schema extends z.ZodSchema ? z.infer<Schema> : undefined;

type apiFetchArgs<
RequestSchema extends z.ZodSchema | undefined,
QueryParamsSchema extends z.ZodSchema | undefined,
ResponseSchema extends z.ZodSchema | undefined,
> = {
config: () => RequestConfig;
path: string;
queryParams?: QP;
queryParams?: SchemaOrUndefined<QueryParamsSchema>;
request?: Omit<RequestInit, "headers" | "body"> & { body?: string };
useSession?: boolean;
bodyRaw?: B;
bodyRaw?: SchemaOrUndefined<RequestSchema>;
requestSchema: RequestSchema;
queryParamsSchema: QueryParamsSchema;
responseSchema: ResponseSchema;
};

type schemaOrUndefined<A> = A extends z.ZodSchema
? z.infer<A>
: never | undefined;

export async function apiFetch(props: {
args: apiFetchArgs<
schemaOrUndefined<typeof props.requestSchema>,
schemaOrUndefined<typeof props.queryParamsSchema>
>;
requestSchema: z.ZodSchema | undefined;
queryParamsSchema: z.ZodSchema | undefined;
responseSchema: z.ZodSchema | undefined;
}): Promise<schemaOrUndefined<typeof props.responseSchema>> {
const { args, requestSchema, queryParamsSchema, responseSchema } = props;
export async function apiFetch<
RequestSchema extends z.ZodSchema | undefined,
QueryParamsSchema extends z.ZodSchema | undefined,
ResponseSchema extends z.ZodSchema | undefined,
>(
args: apiFetchArgs<RequestSchema, QueryParamsSchema, ResponseSchema>
): Promise<SchemaOrUndefined<ResponseSchema>> {
const { requestSchema, queryParamsSchema, responseSchema } = args;

if (requestSchema && args.bodyRaw) {
const parsedRequestBody = requestSchema.safeParse(args.bodyRaw);
if (!parsedRequestBody.success) {
throw new RequestBodyParseError(parsedRequestBody.error);
}
}

if (queryParamsSchema && args.queryParams) {
const parsedQueryParams = queryParamsSchema.safeParse(args.queryParams);
if (!parsedQueryParams.success) {
Expand All @@ -88,8 +93,7 @@ export async function apiFetch(props: {
apiHost: config().apiHost,
sessionId: undefined,
};
// TODO: Query params have stronger types, but they are not just shown here.
// Look into furthering the ensuring of passing proper query params.
const sessionWasUsed = Boolean(usedConfig.sessionId);
const url = getUrl(usedConfig, path, queryParams);

const response = await fetchRetry(url, {
Expand All @@ -101,17 +105,21 @@ export async function apiFetch(props: {
});

if (!response.ok) {
throw await ApiError.createFromResponse(response);
throw await ApiError.createFromResponse(response, {
sessionWasUsed,
});
}

if (responseSchema === undefined) return undefined;
if (responseSchema === undefined) {
return undefined as SchemaOrUndefined<ResponseSchema>;
}

const parsed = responseSchema.safeParse(await response.json());
if (!parsed.success) {
throw new ParseError(parsed.error);
} else {
return parsed.data;
}

return parsed.data;
}

function getAuthHeaders(config: RequestConfig): RequestInit["headers"] {
Expand Down
18 changes: 8 additions & 10 deletions packages/thunderstore-api/src/delete/packageWiki.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
packageWikiPageDeleteRequestDataSchema,
} from "../schemas/requestSchemas";

export async function deletePackageWikiPage(
export function deletePackageWikiPage(
Copy link
Contributor

@anttimaki anttimaki Nov 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

What is the reasoning for removing these async/awaits? Does it change the behaviour anywhere when the apiFetch itself still awaits the request? I'm asking since if the behaviour doesn't change, there's a chance this change was done under the assumption that it would change something, which in part could cause issues elsewhere.

If this is just "removing unnecessary stuff", I guess the changes themself are fine (although a bit pointless), but they definitely shouldn't had be done as part of this huge PR stack. Here they just bloat the amount of review, rebase, etc. (Again, assuming I'm not missing something.)

Copy link
Contributor Author

@Oksamies Oksamies Dec 3, 2025

Choose a reason for hiding this comment

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

Behavior is not changed as it's still the promise that is returned. Fair point that these could have been in a separate PR, not included in the stack.

props: ApiEndpointProps<
PackageWikiPageDeleteRequestParams,
object,
Expand All @@ -16,15 +16,13 @@ export async function deletePackageWikiPage(
const { config, params, data } = props;
const path = `api/experimental/package/${params.namespace_id}/${params.package_name}/wiki/`;

return await apiFetch({
args: {
config: config,
path: path,
request: {
method: "DELETE",
cache: "no-store",
body: JSON.stringify(data),
},
return apiFetch({
config: config,
path: path,
request: {
method: "DELETE",
cache: "no-store",
body: JSON.stringify(data),
},
requestSchema: packageWikiPageDeleteRequestDataSchema,
queryParamsSchema: undefined,
Expand Down
12 changes: 5 additions & 7 deletions packages/thunderstore-api/src/delete/teamDisband.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,12 @@ export function teamDisband(
const path = `/api/cyberstorm/team/${params.team_name}/disband/`;

return apiFetch({
args: {
config,
path,
request: {
method: "DELETE",
},
useSession: true,
config,
path,
request: {
method: "DELETE",
},
useSession: true,
requestSchema: undefined,
queryParamsSchema: undefined,
responseSchema: undefined,
Expand Down
12 changes: 5 additions & 7 deletions packages/thunderstore-api/src/delete/teamRemoveMember.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,12 @@ export function teamRemoveMember(
const path = `/api/cyberstorm/team/${params.team_name}/member/${params.username}/remove/`;

return apiFetch({
args: {
config,
path,
request: {
method: "DELETE",
},
useSession: true,
config,
path,
request: {
method: "DELETE",
},
useSession: true,
requestSchema: undefined,
queryParamsSchema: undefined,
responseSchema: undefined,
Expand Down
12 changes: 5 additions & 7 deletions packages/thunderstore-api/src/delete/teamServiceAccountRemove.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,12 @@ export function teamServiceAccountRemove(
const path = `/api/cyberstorm/service-account/${params.uuid}/delete/`;

return apiFetch({
args: {
config,
path,
request: {
method: "DELETE",
},
useSession: true,
config,
path,
request: {
method: "DELETE",
},
useSession: true,
requestSchema: undefined,
queryParamsSchema: undefined,
responseSchema: undefined,
Expand Down
12 changes: 5 additions & 7 deletions packages/thunderstore-api/src/delete/userDelete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,12 @@ export function userDelete(
const path = `/api/cyberstorm/user/delete/`;

return apiFetch({
args: {
config,
path,
request: {
method: "DELETE",
},
useSession: true,
config,
path,
request: {
method: "DELETE",
},
useSession: true,
requestSchema: undefined,
queryParamsSchema: undefined,
responseSchema: undefined,
Expand Down
12 changes: 5 additions & 7 deletions packages/thunderstore-api/src/delete/userLinkedAccountDisconnect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,12 @@ export function userLinkedAccountDisconnect(
const path = `/api/cyberstorm/user/linked-account/${params.provider}/disconnect/`;

return apiFetch({
args: {
config,
path,
request: {
method: "DELETE",
},
useSession: true,
config,
path,
request: {
method: "DELETE",
},
useSession: true,
requestSchema: userLinkedAccountDisconnectRequestDataSchema,
queryParamsSchema: undefined,
responseSchema: undefined,
Expand Down
Loading
Loading