Skip to content

Commit

Permalink
fix: Resolver not found error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
platosha committed Sep 24, 2024
1 parent 96c03c1 commit 99fa152
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 11 deletions.
13 changes: 11 additions & 2 deletions src/resolver/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,15 @@
import type { EmptyObject } from 'type-fest';
import type { InternalRouteContext, InternalRoute } from '../internal.js';
import type { ActionResult, AnyObject, Route, MaybePromise } from '../types.js';
import { ensureRoutes, getNotFoundError, getRoutePath, isString, notFoundResult, toArray } from '../utils.js';
import {
ensureRoutes,
getNotFoundError,
getRoutePath,
isString,
NotFoundError,
notFoundResult,
toArray
} from '../utils.js';
import matchRoute, { type MatchWithRoute } from './matchRoute.js';
import defaultResolveRoute from './resolveRoute.js';

Expand Down Expand Up @@ -251,7 +259,8 @@ export default class Resolver<R extends AnyObject = EmptyObject> {
}

return await next(true, this.root).catch((error: unknown) => {
const _error = new ResolutionError(currentContext, { code: 500, cause: error });
const _error =
error instanceof NotFoundError ? error : new ResolutionError(currentContext, { code: 500, cause: error });

if (this.errorHandler) {
currentContext.result = this.errorHandler(_error);
Expand Down
21 changes: 12 additions & 9 deletions test/resolver/resolver.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import chaiAsPromised from 'chai-as-promised';
import chaiDom from 'chai-dom';
import sinon from 'sinon';
import sinonChai from 'sinon-chai';
import Resolver from '../../src/resolver/resolver.js';
import Resolver, { ResolutionError } from '../../src/resolver/resolver.js';
import '../setup.js';
import type { AnyObject, RouteContext } from '../../src/types.js';

Expand Down Expand Up @@ -53,10 +53,11 @@ describe('Resolver', () => {
});

it('should support custom error handler option', async () => {
const errorHandler = sinon.spy(() => 'result');
const errorResult = document.createElement('error-result');
const errorHandler = sinon.spy(() => errorResult);
const resolver = new Resolver([], { errorHandler });
const result = await resolver.resolve('/');
expect(result).to.be.equal('result');
expect(result).to.be.equal(errorResult);
expect(errorHandler.calledOnce).to.be.true;
const error = errorHandler.firstCall.firstArg;
expect(error).to.be.an('error');
Expand All @@ -68,7 +69,8 @@ describe('Resolver', () => {
});

it('should handle route errors', async () => {
const errorHandler = sinon.spy(() => 'result');
const errorResult = document.createElement('error-result');
const errorHandler = sinon.spy(() => errorResult);
const route = {
action: () => {
throw new Error('custom');
Expand All @@ -77,12 +79,13 @@ describe('Resolver', () => {
};
const resolver = new Resolver<string>(route, { errorHandler });
const result = await resolver.resolve('/');
expect(result).to.be.equal('result');
expect(result).to.be.equal(errorResult);
expect(errorHandler).to.be.calledOnce;

const error = errorHandler.firstCall.firstArg;
expect(error).to.be.an('error');
expect(error).to.have.property('message').that.equals('custom');
const error: ResolutionError<Error> = errorHandler.firstCall.firstArg;
expect(error).to.be.instanceof(ResolutionError);
expect(error.cause).to.be.an('error');
expect(error.cause).to.have.property('message').that.equals('custom');
expect(error).to.have.property('code', 500);
expect(error).to.have.property('context').that.includes({ pathname: '/', resolver });
expect(error).to.have.nested.property('context.route').that.includes(route);
Expand Down Expand Up @@ -142,7 +145,7 @@ describe('Resolver', () => {
.to.have.property('message')
.that.matches(/Page not found/u);
expect(error).to.have.property('code').that.equals(404);
expect(error).to.have.property('context').that.includes({ path: undefined, pathname: '/', resolver });
expect(error).to.have.property('context').that.includes({ pathname: '/', resolver });
});

it("should execute the matching route's action method and return its result", async () => {
Expand Down

0 comments on commit 99fa152

Please sign in to comment.