shardeum / shardus-core

Other
10 stars 2 forks source link

external endpoint timeout guard #246

Closed S0naliThakur closed 3 weeks ago

S0naliThakur commented 3 weeks ago

Linear: https://linear.app/shm/issue/PRI-41 Summary: Added middleware to enforce a timeout on external endpoints, preventing waits beyond a set time limit.

github-actions[bot] commented 3 weeks ago

PR Reviewer Guide ๐Ÿ”

โฑ๏ธ Estimated effort to review: 3 ๐Ÿ”ต๐Ÿ”ต๐Ÿ”ตโšชโšช
๐Ÿงช No relevant tests
๐Ÿ”’ No security concerns identified
โšก Key issues to review

Possible Memory Leak
The `timeoutMiddleware` function sets a timeout using `setTimeout` but does not always clear it if the response is sent before the timeout. This could potentially lead to memory leaks if the `clearTimer` function is not called in all cases where the response is handled. Redundant Code
The `customSendJsonMiddleware` function has redundant checks for `res.headersSent` which are already handled in the `res.send` and `res.json` methods. This could be simplified to avoid unnecessary checks and improve code readability.
kgmyatthu commented 3 weeks ago

This can be done in much simpler way. Please use express's builtin support for this. Something like this is all you need. Example:

const server = app.listen()
server.setTimeout(5000)

Current implementation in PR add unnecessary LOC(s). Please modify.