Add the json-deserializer middleware package

[matt-jenner]

This implements a json-deserializer middleware, it expects an object extending ApiGatewayProxyEvent and returns an ApiGatewayProxyObjectEvent which replaces the string body for an object of type Record<string, unknown>.

It introduces the concept of a RequestBodyNotJsonError to be used to catch and return an appropriate 400 error back if a non json payload is passed.

If no body or a non-json header is passed it will pass null as the body property on the event passed to the underlying handler.

Closes #54

Checklist:

• [x] I have updated the documentation accordingly.
• [x] I have read the CONTRIBUTING document.

I'm sorry for the delays in supplying this PR, but things got crazily busy at work, and then summer (and a large DIY project - building a garden office now my work has decided no-one is going back to the office full-time) happened which I'm afraid took most of my spare time and I like to code when fully able to commit myself to it.

Although I mostly got this done within a few hours work. I ran into an issue with the 100% code-coverage requirements which caused me to delay submiting. While I do have full coverage in TS (and I always like to see with libraries I expect someone to depend upon), there was one issue where the super(message) line in the RequestBodyNotJsonError class would NOT mark itself as covered despite definately hitting the line in my tests.

Jest reports the line as not-covered due to the coverage being applied to ES5 code rather than the TS itself. I've confirmed coverage is 100% as if you switch the output to ES6 it reports 100% coverage.

After reading quite a bit around the issue, it seems to relate to the fact that when the TS line: super(message)

is transpiled down to ES5, it comes out as: var _this = _super.call(this, message) || this;

While the former part of the or statement is covered, it doesn't cover all the cases of the transpiled statement fully.

I've tried various approaches to resolve this including a suggestion of adding https://www.npmjs.com/package/ts-es5-istanbul-coverage but eventually thought it best to submit it cleanly without this and see your thoughts.

See here for more information:

• https://github.com/microsoft/TypeScript/issues/13029
• https://github.com/SitePen/remap-istanbul/issues/106

Otherwise, hopefully the PR should be otherwise mostly self-explanatory, the logic itself is very simple and is the precursor for my other feature request logged in #55.

[CLAassistant]

All committers have signed the CLA.

[dbartholomae]

Concerning the coverage, just adding a comment explaining it and an Istanbul ignore next might well suffice

[dbartholomae]

Please also rebase, this should run CI for the branch (I've just updated the main branch with a small config change).

[codecov[bot]]

Codecov Report

Merging #57 (dfa8dbd) into main (30b8e5e) will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##              main       #57   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           54        54           
  Lines          520       520           
  Branches        98        98           
=========================================
  Hits           520       520           

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 30b8e5e...dfa8dbd. Read the comment docs.

[matt-jenner]

Thanks for the review, I hope you had a good holiday. I've added your suggestions and fixed one codeQL vulnerability relating to Regular expression denial of service (it was simpler to just remove the regex 🙂).

It's running the build now but for some reason it's not seeing my package in the build, it lists 13 packages not the 14 I see if I run rush test:integration or rush test:unit locally and the project is listed in rush.json so I would have thought it would have picked it up?

Any ideas on this front as I'm afraid I'm not a rush expert, thanks 🙂.

[matt-jenner]

Thanks for the approval - it will feel good to finally get this in, I think you need to merge this manually though as mergify is complaining that the workflow has changed. Once this is in, I can start on the next issue I logged to create the io-ts validator middleware before I'm due back in work next week, thanks 🙂.

[dbartholomae]

Thanks! I now figured out the problem: When I changed the settings to run the GitHub actions for this PR, I changed them in a way that lead to actions running on the main branch code instead of the PR branch code instead. I've just updated the main branch to fix this. If you could rebase/merge main one more time, it should be solved :)

[dbartholomae]

Just realized that you gave me access to the branch, so I did the merge of main myself and will merge this branch once CI is green.

[dbartholomae]

There was also a slight problem with the integration test: it still used body instead of bodyObject. It's fixed, and version 1.1.0 is now available on npm.

[matt-jenner]

There was also a slight problem with the integration test: it still used body instead of bodyObject. It's fixed, and version 1.1.0 is now available on npm.

Thanks for fixing that, I really mustn't code quite so late 🙂 looking forward to switching my project over to use this now and start on the next middleware 😁