Skip to content

Commit

Permalink
fix: return 404 instead of 500 for missing collections
Browse files Browse the repository at this point in the history
- Replace string rejection with TaxiiNotFoundException in repository
- Remove Promise constructor anti-pattern in findOne method
- Add debug logging for collection lookups
- Update JSDoc to document exception behavior

The /collections/{id} endpoint now correctly returns a 404 status
when a collection is not found in the database, rather than a 500
internal server error.
  • Loading branch information
seansica committed Nov 17, 2024
1 parent e3530b9 commit e9369d6
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 49 deletions.
80 changes: 39 additions & 41 deletions src/common/exceptions/taxii-exception.filter.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {
ArgumentsHost,
ExceptionFilter,
HttpException,
Logger,
} from "@nestjs/common";
import { Response } from "express";
Expand All @@ -12,61 +11,60 @@ import {
RequestContextModel,
} from "../middleware/request-context";

// @Catch(MyException) => can set to only catch specific exception types!
export class TaxiiExceptionFilter implements ExceptionFilter {
private readonly logger: Logger = new Logger();
private readonly logger: Logger = new Logger(TaxiiExceptionFilter.name);

/**
* Handles uncaught exceptions. Any exceptions which are caught that are not a subclass of TaxiiErrorException are
* transformed to instances of TaxiiInternalServerErrorException and returned to the user.
* @param exception The uncaught exception instance that triggered the TaxiiExceptionFilter
* @param host Provides access to the HTTP context so that the Request and Response objects can be worked upon
* @param exception The uncaught exception instance that triggered the filter
* @param host Provides access to the HTTP context
*/
catch(exception: HttpException, host: ArgumentsHost): any {
catch(exception: unknown, host: ArgumentsHost): any {
const ctx = host.switchToHttp();
const response = ctx.getResponse<Response>();
const reqCtx: RequestContext = RequestContextModel.get();
const requestId = reqCtx?.["x-request-id"] || "unknown";

// Log the exception type and details
this.logger.error(
`[${reqCtx["x-request-id"]}] The exception type is: ${
exception.constructor.name
}. The exception is ${JSON.stringify(exception)}`
`[${requestId}] Exception occurred: ${exception?.constructor?.name || typeof exception}`
);

// Send a generic 500 error for any uncaught exceptions
if (!(exception instanceof TaxiiErrorException)) {
this.logger.error(
`[${reqCtx["x-request-id"]}] Received an unhandled exception. Please investigate the root cause of the exception triggered by request: ${reqCtx["x-request-id"]}`
);
this.logger.error(
`[${reqCtx["x-request-id"]}] The error is ${exception.stack.toString()}`
);
const internalServerError: TaxiiInternalServerErrorException =
new TaxiiInternalServerErrorException({
title: "Internal Error",
description:
"An unexpected error has occurred. Please contact the TAXII server administrator.",
// errorId: request['x-request-id'].toString(),
errorId: reqCtx["x-request-id"].toString(),
});
if (typeof exception === 'string') {
this.logger.error(`[${requestId}] String exception: ${exception}`);
} else if (exception instanceof Error) {
this.logger.error(`[${requestId}] Error details: ${exception.message}`);
if (exception.stack) {
this.logger.error(`[${requestId}] Stack trace: ${exception.stack}`);
}
} else {
try {
this.logger.error(
`[${requestId}] Exception details: ${JSON.stringify(exception)}`
);
} catch (e) {
this.logger.error(
`[${requestId}] Could not stringify exception: ${e.message}`
);
}
}

// Send the error
response
.status(TaxiiInternalServerErrorException.httpStatus)
.json(internalServerError);
// Handle TAXII exceptions
if (exception instanceof TaxiiErrorException) {
exception.errorId = requestId.toString();
return response.status(exception.httpStatus).json(exception);
}

// The else clause is triggered only if the exception type if one of the custom Taxii exception classes, in
// which case all we want to do is set the errorId field then return the exception.
else {
// Copy the request's unique ID onto the outgoing response body
exception.errorId = reqCtx["x-request-id"].toString();
// For all other exceptions (including strings, non-HTTP errors, etc.)
const internalServerError = new TaxiiInternalServerErrorException({
title: "Internal Error",
description: "An unexpected error has occurred. Please contact the TAXII server administrator.",
errorId: requestId.toString(),
});

// Send the error
response
//.set('id', request['id'])
.status(exception.httpStatus)
.json(exception);
}
return response
.status(TaxiiInternalServerErrorException.httpStatus)
.json(internalServerError);
}
}
}
20 changes: 12 additions & 8 deletions src/taxii/providers/collection/collection.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
TaxiiCollectionDocument,
} from "src/hydrate/schema";
import { Model } from "mongoose";
import { isNull } from "lodash";
import { TaxiiNotFoundException } from "src/common/exceptions";

@Injectable()
export class CollectionRepository {
Expand All @@ -24,9 +24,11 @@ export class CollectionRepository {
*
* @param id TAXII/STIX ID of the collection
* @returns Promise resolving to TaxiiCollectionDto
* @throws If collection is not found
* @throws TaxiiNotFoundException if collection is not found
*/
async findOne(id: string): Promise<TaxiiCollectionDto> {
this.logger.debug(`Searching for collection with ID: ${id}`);

// Uses taxii_collection_lookup index
const response: TaxiiCollectionEntity = await this.collectionModel
.findOne({
Expand All @@ -35,12 +37,14 @@ export class CollectionRepository {
})
.exec();

return new Promise((resolve, reject) => {
if (!isNull(response)) {
resolve(new TaxiiCollectionDto({ ...response["_doc"] }));
}
reject(`Collection ID '${id}' not available in database`);
});
if (!response) {
throw new TaxiiNotFoundException({
title: "Collection Not Found",
description: `Collection ID '${id}' not available in database`,
});
}

return new TaxiiCollectionDto({ ...response["_doc"] });
}

/**
Expand Down

0 comments on commit e9369d6

Please sign in to comment.