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

ARC-1429: Return 404 if Jira has been renamed. #1268

Merged
merged 9 commits into from
Jun 16, 2022
18 changes: 17 additions & 1 deletion src/jira/client/axios.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,23 @@ describe("Jira axios instance", () => {
}

expect(error?.status).toEqual(404);
expect(error?.message).toEqual("Error executing Axios Request HTTP 404 - Bad REST path, or Jira instance not found or temporarily suspended.");
expect(error?.message).toEqual("Error executing Axios Request HTTP 404 - Bad REST path, or Jira instance not found, renamed or temporarily suspended.");
});

it("should return 404 from failed request if Jira has been renamed", async () => {
const requestPayload = "TestRequestPayload";
jiraNock.post("/foo/bar", requestPayload).reply(405);
jiraNock.get("/status").reply(302);

let error;
try {
await getAxiosInstance(jiraHost, "secret").post("/foo/bar", requestPayload);
} catch (e) {
error = e;
}

expect(error?.status).toEqual(404);
expect(error?.message).toEqual("Error executing Axios Request HTTP 404 - Bad REST path, or Jira instance not found, renamed or temporarily suspended.");
});

});
12 changes: 11 additions & 1 deletion src/jira/client/axios.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export const getJiraErrorMessages = (status: number) => {
case 403:
return "HTTP 403 - The JWT token used does not correspond to an app that defines the jiraDevelopmentTool module, or the app does not define the 'WRITE' scope";
case 404:
return "HTTP 404 - Bad REST path, or Jira instance not found or temporarily suspended.";
return "HTTP 404 - Bad REST path, or Jira instance not found, renamed or temporarily suspended.";
case 413:
return "HTTP 413 - Data is too large. Submit fewer devinfo entities in each payload.";
case 429:
Expand Down Expand Up @@ -157,6 +157,16 @@ const instrumentFailedRequest = (baseURL: string, logger: Logger) => {
}
}
}
if (error.response?.status === 405) {
try {
await axios.get("/status", { baseURL });
} catch (e) {
if (e.response.status === 302) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: use else/if instead of if

Copy link
Contributor Author

@atrigueiro atrigueiro Jun 15, 2022

Choose a reason for hiding this comment

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

If I use else if I will have to define another else at the end, but there's nothing else I want to do there. Do I just do else { return; } ?

Copy link
Contributor

Choose a reason for hiding this comment

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

else if doesn't force to have else at the end, so skip else if not required

Copy link
Contributor Author

@atrigueiro atrigueiro Jun 16, 2022

Choose a reason for hiding this comment

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

Ooo nice I didn't know that, I thought else was required. Thanks Harminder, just updated :)

logger.info(`405 from Jira: Jira instance '${baseURL}' has been renamed. Returning 404 to our application.`);
error.response.status = 404;
}
}
}
return Promise.reject(error);
mboudreau marked this conversation as resolved.
Show resolved Hide resolved
};
};
Expand Down