fix: Response wrapping, error logging, and controller hardening (COA 3)
All checks were successful
Test Asgard Runner / test (push) Successful in 3s
All checks were successful
Test Asgard Runner / test (push) Successful in 3s
- HttpExceptionFilter: Log actual error details for non-HttpExceptions (was silently swallowing 500s)
- ServersService: Return null fields instead of 404 for new licenses without servers
- NotificationsController: Wrap config responses as { config } to match frontend expectations
- WebstoreController: Wrap config responses as { config } to match frontend expectations
- ChatController: Replace ParseIntPipe with manual parseInt (400 on missing optional param)
- WipesController: Same ParseIntPipe fix for history limit param
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -4,14 +4,18 @@ import {
|
|||||||
ArgumentsHost,
|
ArgumentsHost,
|
||||||
HttpException,
|
HttpException,
|
||||||
HttpStatus,
|
HttpStatus,
|
||||||
|
Logger,
|
||||||
} from '@nestjs/common';
|
} from '@nestjs/common';
|
||||||
import { Response } from 'express';
|
import { Response } from 'express';
|
||||||
|
|
||||||
@Catch()
|
@Catch()
|
||||||
export class HttpExceptionFilter implements ExceptionFilter {
|
export class HttpExceptionFilter implements ExceptionFilter {
|
||||||
|
private readonly logger = new Logger('ExceptionFilter');
|
||||||
|
|
||||||
catch(exception: unknown, host: ArgumentsHost) {
|
catch(exception: unknown, host: ArgumentsHost) {
|
||||||
const ctx = host.switchToHttp();
|
const ctx = host.switchToHttp();
|
||||||
const response = ctx.getResponse<Response>();
|
const response = ctx.getResponse<Response>();
|
||||||
|
const request = ctx.getRequest();
|
||||||
|
|
||||||
let status = HttpStatus.INTERNAL_SERVER_ERROR;
|
let status = HttpStatus.INTERNAL_SERVER_ERROR;
|
||||||
let message = 'Internal server error';
|
let message = 'Internal server error';
|
||||||
@@ -28,6 +32,13 @@ export class HttpExceptionFilter implements ExceptionFilter {
|
|||||||
message = obj.message[0] as string;
|
message = obj.message[0] as string;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
// Log non-HttpException errors with full details
|
||||||
|
const err = exception instanceof Error ? exception : new Error(String(exception));
|
||||||
|
this.logger.error(
|
||||||
|
`Unhandled exception on ${request.method} ${request.url}: ${err.message}`,
|
||||||
|
err.stack,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
response.status(status).json({ message });
|
response.status(status).json({ message });
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { Controller, Get, Put, Param, Body, Query, ParseIntPipe, UseGuards } from '@nestjs/common';
|
import { Controller, Get, Put, Param, Body, Query, UseGuards } from '@nestjs/common';
|
||||||
import { ApiTags, ApiBearerAuth, ApiOperation, ApiQuery } from '@nestjs/swagger';
|
import { ApiTags, ApiBearerAuth, ApiOperation, ApiQuery } from '@nestjs/swagger';
|
||||||
import { ChatService } from './chat.service';
|
import { ChatService } from './chat.service';
|
||||||
import { FlagMessageDto } from './dto/flag-message.dto';
|
import { FlagMessageDto } from './dto/flag-message.dto';
|
||||||
@@ -21,9 +21,10 @@ export class ChatController {
|
|||||||
@ApiQuery({ name: 'limit', required: false, example: 100 })
|
@ApiQuery({ name: 'limit', required: false, example: 100 })
|
||||||
async getMessages(
|
async getMessages(
|
||||||
@CurrentTenant() licenseId: string,
|
@CurrentTenant() licenseId: string,
|
||||||
@Query('limit', new ParseIntPipe({ optional: true })) limit?: number,
|
@Query('limit') limit?: string,
|
||||||
) {
|
) {
|
||||||
return await this.chatService.getMessages(licenseId, limit || 100);
|
const limitNum = limit ? parseInt(limit, 10) : 100;
|
||||||
|
return await this.chatService.getMessages(licenseId, limitNum);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Put(':id/flag')
|
@Put(':id/flag')
|
||||||
|
|||||||
@@ -27,7 +27,8 @@ export class NotificationsController {
|
|||||||
description: 'Notification config retrieved successfully',
|
description: 'Notification config retrieved successfully',
|
||||||
})
|
})
|
||||||
async getConfig(@CurrentTenant() licenseId: string) {
|
async getConfig(@CurrentTenant() licenseId: string) {
|
||||||
return await this.notificationsService.getConfig(licenseId);
|
const config = await this.notificationsService.getConfig(licenseId);
|
||||||
|
return { config };
|
||||||
}
|
}
|
||||||
|
|
||||||
@Put('config')
|
@Put('config')
|
||||||
@@ -43,6 +44,7 @@ export class NotificationsController {
|
|||||||
@CurrentTenant() licenseId: string,
|
@CurrentTenant() licenseId: string,
|
||||||
@Body() dto: UpdateConfigDto,
|
@Body() dto: UpdateConfigDto,
|
||||||
) {
|
) {
|
||||||
return await this.notificationsService.updateConfig(licenseId, dto);
|
const config = await this.notificationsService.updateConfig(licenseId, dto);
|
||||||
|
return { config };
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -17,7 +17,8 @@ export class ServersService {
|
|||||||
) {}
|
) {}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Get server connection and config for a license
|
* Get server connection and config for a license.
|
||||||
|
* Returns null fields if no server has been set up yet.
|
||||||
*/
|
*/
|
||||||
async getServer(licenseId: string) {
|
async getServer(licenseId: string) {
|
||||||
const connection = await this.connectionRepo.findOne({
|
const connection = await this.connectionRepo.findOne({
|
||||||
@@ -28,11 +29,11 @@ export class ServersService {
|
|||||||
where: { license_id: licenseId },
|
where: { license_id: licenseId },
|
||||||
});
|
});
|
||||||
|
|
||||||
if (!connection || !config) {
|
return {
|
||||||
throw new NotFoundException('Server not found for this license');
|
connection: connection || null,
|
||||||
}
|
config: config || null,
|
||||||
|
setup_required: !connection || !config,
|
||||||
return { connection, config };
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -18,7 +18,8 @@ export class WebstoreController {
|
|||||||
@ApiBearerAuth()
|
@ApiBearerAuth()
|
||||||
@ApiOperation({ summary: 'Get webstore configuration' })
|
@ApiOperation({ summary: 'Get webstore configuration' })
|
||||||
async getConfig(@CurrentTenant() licenseId: string) {
|
async getConfig(@CurrentTenant() licenseId: string) {
|
||||||
return this.webstoreService.getConfig(licenseId);
|
const config = await this.webstoreService.getConfig(licenseId);
|
||||||
|
return { config };
|
||||||
}
|
}
|
||||||
|
|
||||||
@Put('webstore/config')
|
@Put('webstore/config')
|
||||||
@@ -28,7 +29,8 @@ export class WebstoreController {
|
|||||||
@CurrentTenant() licenseId: string,
|
@CurrentTenant() licenseId: string,
|
||||||
@Body() dto: UpdateStoreConfigDto,
|
@Body() dto: UpdateStoreConfigDto,
|
||||||
) {
|
) {
|
||||||
return this.webstoreService.updateConfig(licenseId, dto);
|
const config = await this.webstoreService.updateConfig(licenseId, dto);
|
||||||
|
return { config };
|
||||||
}
|
}
|
||||||
|
|
||||||
@Get('webstore/categories')
|
@Get('webstore/categories')
|
||||||
|
|||||||
@@ -7,7 +7,6 @@ import {
|
|||||||
Body,
|
Body,
|
||||||
Param,
|
Param,
|
||||||
Query,
|
Query,
|
||||||
ParseIntPipe,
|
|
||||||
UseGuards,
|
UseGuards,
|
||||||
} from '@nestjs/common';
|
} from '@nestjs/common';
|
||||||
import { ApiTags, ApiBearerAuth, ApiOperation, ApiQuery } from '@nestjs/swagger';
|
import { ApiTags, ApiBearerAuth, ApiOperation, ApiQuery } from '@nestjs/swagger';
|
||||||
@@ -81,9 +80,10 @@ export class WipesController {
|
|||||||
@ApiQuery({ name: 'limit', required: false, example: 50 })
|
@ApiQuery({ name: 'limit', required: false, example: 50 })
|
||||||
getHistory(
|
getHistory(
|
||||||
@CurrentTenant() licenseId: string,
|
@CurrentTenant() licenseId: string,
|
||||||
@Query('limit', new ParseIntPipe({ optional: true })) limit?: number,
|
@Query('limit') limit?: string,
|
||||||
) {
|
) {
|
||||||
return this.wipesService.getHistory(licenseId, limit || 50);
|
const limitNum = limit ? parseInt(limit, 10) : 50;
|
||||||
|
return this.wipesService.getHistory(licenseId, limitNum);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Post('trigger')
|
@Post('trigger')
|
||||||
|
|||||||
Reference in New Issue
Block a user