Skip to content

fix: correctly handle other errors from handlers #1033

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

Closed
jpulec wants to merge 1 commit into from
Closed

fix: correctly handle other errors from handlers #1033

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 12 additions & 3 deletions src/handlers/handlerUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@ export async function tryTargetsRecursively(
];
// end: merge inherited config with current target config (preference given to current)

let response;
let response: Response | null = null;

switch (strategyMode) {
case StrategyModes.FALLBACK:
Expand Down Expand Up @@ -859,7 +859,7 @@ export async function tryTargetsRecursively(
currentJsonPath,
method
);
} catch (error: any) {
} catch (error: unknown) {
Copy link
Collaborator

@narengogi narengogi Apr 21, 2025

Choose a reason for hiding this comment

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

I've just gone through this code, and I think this catch block is meant to set a response object in all cases.
so the correct fix to make sure all cases are handled would be

if (typeof error === 'object' &&
      error &&
          'response' in error &&
          error.response instanceof Response
        ) {
          response = error.response;
        } else if (error instanceof GatewayError) {...} else {response = new Response(500,...)}

Copy link
Collaborator

@narengogi narengogi Apr 21, 2025

Choose a reason for hiding this comment

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

@VisargD to take a call here

// tryPost always returns a Response.
// TypeError will check for all unhandled exceptions.
// GatewayError will check for all handled exceptions which cannot allow the request to proceed.
Expand All @@ -882,13 +882,22 @@ export async function tryTargetsRecursively(
},
}
);
} else {
} else if (
typeof error === 'object' &&
error &&
'response' in error &&
error.response instanceof Response
) {
response = error.response;
}
}
break;
}

if (!response) {
throw new Error('Could not create response');
}

return response;
}

Expand Down