Skip to content

Conversation

@kastov
Copy link
Collaborator

@kastov kastov commented May 21, 2025

Release 1.5.2 with the addition of the compression middleware and updated package dependencies. Key changes include:

  • Added 'compression' package to improve response compression in the application.
  • Updated version in package.json to 1.5.2.
  • Enhanced start message to clarify internal ports used.

kastov added 2 commits May 21, 2025 20:06
Release 1.5.2 with the addition of the compression middleware and updated package dependencies. Key changes include:
- Added 'compression' package to improve response compression in the application.
- Updated version in package.json to 1.5.2.
- Enhanced start message to clarify internal ports used.
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This release (v1.5.2) adds compression middleware and updates internal port display in startup messages.

  • Added compression middleware in src/main.ts but missing implementation for internal app instance
  • Fixed hardcoded ports (61000, internalPort, 61002) in src/common/utils/get-start-message.ts startup display
  • Incorrect error constant in src/modules/stats/stats.service.ts (FAILED_TO_GET_INBOUNDS_STATS vs FAILED_TO_GET_OUTBOUNDS_STATS)
  • Leftover commented demo code in getUsersStats method should be removed
  • Inconsistent error logging patterns across stats.service.ts error handlers

4 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings | Greptile

[
['Docs → https://remna.st\nCommunity → https://t.me/remnawave'],
[`API Port: ${appPort}\nInternal Port: ${internalPort}`],
[`API Port: ${appPort}\nInternal Ports: 61000, ${internalPort}, 61002`],
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Hardcoding port numbers (61000, 61002) in the message could cause confusion if actual ports change. Consider using constants or configuration values instead.

}),
});

app.use(json({ limit: '1000mb' }));
Copy link
Contributor

Choose a reason for hiding this comment

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

style: 1000mb limit may be excessive and could expose the server to DoS attacks. Consider a lower limit based on actual requirements

Comment on lines +96 to +105
// const demoRes = Array.from({ length: 160_000 }, (_, i) => ({
// username: String(i + 1),
// uplink: Math.floor(Math.random() * (107374182400 - 10485760) + 10485760), // Random between 10MB and 100GB
// downlink: Math.floor(Math.random() * (107374182400 - 10485760) + 10485760), // Random between 10MB and 100GB
// }));

// return {
// isOk: true,
// response: new GetUsersStatsResponseModel(demoRes),
// };
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Remove commented-out demo code before merging to main. If needed for testing, move to a separate test file.

"@remnawave/supervisord-nestjs": "0.1.1",
"@remnawave/xtls-sdk": "0.3.0",
"@remnawave/xtls-sdk-nestjs": "0.2.2",
"compression": "^1.8.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

style: consider pinning compression version to exact number (1.8.0) for better dependency predictability, matching style of other core dependencies

"devDependencies": {
"@nestjs/cli": "11.0.5",
"@nestjs/schematics": "11.0.2",
"@types/compression": "^1.8.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

style: consider pinning @types/compression version to match compression version exactly

"compression": "^1.8.0",
"enhanced-ms": "^4.1.0",
"helmet": "^8.1.0",
"husky": "9.1.7",
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: husky should be in devDependencies since it's only used during development

Suggested change
"husky": "9.1.7",

@kastov kastov merged commit d3082de into main May 21, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants