[#IP-82] Add a response header containing the current app version #140
Conversation
Example of PR titles that include pivotal stories:
New dependencies added: jest-expressAuthor: James W. Lane Description: jest mock Homepage: https://github.com/jameswlane/jest-express#readme
|
Codecov Report
@@ Coverage Diff @@
## master #140 +/- ##
==========================================
+ Coverage 84.43% 85.02% +0.59%
==========================================
Files 33 34 +1
Lines 880 888 +8
Branches 86 91 +5
==========================================
+ Hits 743 755 +12
+ Misses 134 130 -4
Partials 3 3
Continue to review full report at Codecov.
|
src/utils/express.ts
Outdated
| res, | ||
| next | ||
| ): void => { | ||
| fromNullable(process.env.npm_package_version).map(v => |
There was a problem hiding this comment.
I cannot find that in docs, but I'm pretty sure process.env.npm_package_version is defined only when the application is started with npm start (or yarn start).
Although I think that's the case of every app we make, it's a bold assumption. A safer approach would be to read package.json file
import { version } from "../package.json";There was a problem hiding this comment.
Yes, the npm_package_version works only for npm-started app, but using the import approach delegate the "version" extraction to the middleware caller becouse it depends of the project structure ("../package.json").
We can easly take the verison as input in this function, attributing the version rightness responsability to the caller.
Do you think it's safer in this way?
There was a problem hiding this comment.
Good point, I didn't realize that we were going to do that from a package.
We could pass the whole package to the factory method, or we can just read the file from
`${process.cwd()}/package.json`Maybe the second option will provide the right level of abstraction. I don't have a strong opinion actually.
There was a problem hiding this comment.
I've chained both the solution: the verison is extracted by package.json, if fails it will fallback to npm env version.
Changed configureDefaultHandlers to ...Middlewares
…v and process.cwd package.json file
fabriziopapi
requested review from
AleDore,
BurnedMarshal,
francescopersico and
gunzip
as code owners
In order to ease the troubleshooting, we want to add to all the service response a new header field (X-API-Version) containing the current app version.
We created a middleware setting the response header and a common register function to call from all the service code.
Both those functions have been implemented in this project to match the DRY principle and to ease the future similar task.