Skip to content

Commit

Permalink
fix: swallowed exceptions from improper exports
Browse files Browse the repository at this point in the history
Adapters were causing multiple instances of OAuthExceptions to be created, one for each instance.
Instead of going down the bundler rabbit hole, I was thinking about how to fix this, and I realized
that the simplest way is to add a marker on the OAuthException (OAuthException.oauth) and check for
it instead of using instance of. Resolves #82

#82 (comment)
  • Loading branch information
jasonraimondi committed Jul 26, 2024
1 parent 580922f commit 9ff4e7c
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 3 deletions.
3 changes: 2 additions & 1 deletion src/adapters/express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { OAuthException } from "../exceptions/oauth.exception.js";

import { OAuthRequest } from "../requests/request.js";
import { OAuthResponse } from "../responses/response.js";
import { isOAuthError } from "../utils/errors.js";

export function responseFromExpress(res: Response): OAuthResponse {
return new OAuthResponse(res);
Expand All @@ -25,7 +26,7 @@ export function handleExpressResponse(expressResponse: Response, oauthResponse:

// @todo v4.0 flip these to always be Express as first arg, OAuth as second. Then update Docs
export function handleExpressError(e: unknown | OAuthException, res: Response): void {
if (e instanceof OAuthException) {
if (isOAuthError(e)) {
res.status(e.status);
res.send({
status: e.status,
Expand Down
3 changes: 2 additions & 1 deletion src/adapters/fastify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { OAuthException } from "../exceptions/oauth.exception.js";

import { OAuthRequest } from "../requests/request.js";
import { OAuthResponse } from "../responses/response.js";
import { isOAuthError } from "../utils/errors.js";

export function responseFromFastify(res: FastifyReply): OAuthResponse {
return new OAuthResponse({
Expand All @@ -20,7 +21,7 @@ export function requestFromFastify(req: FastifyRequest): OAuthRequest {

// @todo v4.0 flip these to always be Fastify as first arg, OAuth as second. Then update Docs
export function handleFastifyError(e: unknown | OAuthException, res: FastifyReply): void {
if (e instanceof OAuthException) {
if (isOAuthError(e)) {
res.status(e.status).send({
status: e.status,
message: e.message,
Expand Down
6 changes: 6 additions & 0 deletions src/exceptions/oauth.exception.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export enum ErrorType {
}

export class OAuthException extends Error {
private readonly oauth = true;

constructor(
public readonly error: string,
public readonly errorType: ErrorType,
Expand Down Expand Up @@ -123,4 +125,8 @@ export class OAuthException extends Error {
HttpStatus.INTERNAL_SERVER_ERROR,
);
}

get isOAuth(): boolean {
return this.oauth;
}
}
7 changes: 7 additions & 0 deletions src/utils/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { OAuthException } from "../exceptions/oauth.exception.js";

export function isOAuthError(error: unknown): error is OAuthException {
if (!error) return false;
if (typeof error !== "object") return false;
return "oauth" in error;
}
59 changes: 59 additions & 0 deletions test/e2e/adapters/fastify.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { describe, it, expect, vi } from "vitest";
import { FastifyReply, FastifyRequest } from "fastify";
import { handleFastifyError, requestFromFastify, responseFromFastify } from "../../../src/adapters/fastify.js";
import { ErrorType, OAuthException, OAuthRequest, OAuthResponse } from "../../../src/index.js";

describe("Fastify OAuth Utilities", () => {
describe("responseFromFastify", () => {
it("should create an OAuthResponse from FastifyReply", () => {
const mockReply = {
headers: { "Content-Type": "application/json" },
} as unknown as FastifyReply;

const result = responseFromFastify(mockReply);
expect(result).toBeInstanceOf(OAuthResponse);
expect(result.headers).toEqual({ "Content-Type": "application/json" });
});
});

describe("requestFromFastify", () => {
it("should create an OAuthRequest from FastifyRequest", () => {
const mockRequest = {
query: { code: "abc123" },
body: { grant_type: "authorization_code" },
headers: { "Content-Type": "application/json" },
} as unknown as FastifyRequest;

const result = requestFromFastify(mockRequest);
expect(result).toBeInstanceOf(OAuthRequest);
expect(result.query).toEqual({ code: "abc123" });
expect(result.body).toEqual({ grant_type: "authorization_code" });
expect(result.headers).toEqual({ "Content-Type": "application/json" });
});
});

describe("handleFastifyError", () => {
it("should handle OAuthException", () => {
const mockReply = {
status: vi.fn().mockReturnThis(),
send: vi.fn(),
} as unknown as FastifyReply;

const oauthError = new OAuthException("Test error", ErrorType.InternalServerError);
handleFastifyError(oauthError, mockReply);

expect(mockReply.status).toHaveBeenCalledWith(400);
expect(mockReply.send).toHaveBeenCalledWith({
status: 400,
message: "Test error",
});
});

it("should throw non-OAuthException errors", () => {
const mockReply = {} as FastifyReply;
const error = new Error("Regular error");

expect(() => handleFastifyError(error, mockReply)).toThrow("Regular error");
});
});
});
2 changes: 1 addition & 1 deletion vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ export default defineConfig({
exclude: [".github/**", ".idea/**", "docs/**", "example/**"],
},
setupFiles: ["test/setup.ts"],
exclude: ["node_modules/**", "example/**", "version-check.ts"],
exclude: ["docs/**", "example/**", "node_modules/**", "version-check.ts"],
},
});

0 comments on commit 9ff4e7c

Please sign in to comment.