Fix #1805 Include headers in the request object of custom route handlers

[zimeg]

Summary

The changes in this PR assign req.headers to the request object in handlers of custom routes and adds an example to the docs. Fixes #1805.

Reviewers

With the changes on this branch, verify that request headers are displayed in static and dynamic routes using both Socket Mode and HTTP. You can check this by navigating to these routes with the following code:

• http://localhost:3000/health
• http://localhost:3000/music/jazz

const app = new App({
  token: process.env.SLACK_BOT_TOKEN,
  appToken: process.env.SLACK_APP_TOKEN,
  // signingSecret: process.env.SLACK_SIGNING_SECRET,
  socketMode: true,
  customRoutes: [
    {
      method: "GET",
      path: "/health",
      handler: (req, res) => {
        console.log("req.headers: ", req.headers);
        console.log("req.headers.host: ", req.headers.host);
        console.log("req.params: ", req.params);
        res.writeHead(200);
        res.end(`Things are going just fine at ${req.headers.host}!`);
      }
    },
    {
      method: "GET",
      path: "/music/:genre",
      handler: (req, res) => {
        console.log("req.headers: ", req.headers);
        console.log("req.headers.host: ", req.headers.host);
        res.writeHead(200);
        res.end(`Oh? ${req.params.genre}?`);
      }
    }
  ],
});

(async () => {
  await app.start(3000);
  console.log('⚡️ Bolt app is running!');
})();

Diagnostic notes

For those curious about the changes, some findings are below. This was a neat exploration into the Symbol object and how objects are spread! So...

Access to certain attributes of the req object (such as req.headers) was lost in the spread operator in the line below:

this.routes[pathMatch][method]({ ...req, params } as ParamsIncomingMessage, res);

However, similar attributes would appear when logging the req object, just in a Symbol form, like so:

console.log(req);
//   ...
//   [Symbol(kHeaders)]: {
//     host: 'localhost:3000',
//     'sec-fetch-site': 'none',
//     connection: 'keep-alive',
//     'upgrade-insecure-requests': '1',
//     'sec-fetch-mode': 'navigate',
//     accept: 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
//     'user-agent': '...',
//     'accept-language': 'en-US,en;q=0.9',
//     'sec-fetch-dest': 'document',
//     'accept-encoding': 'gzip, deflate'
//   },
//   ...

As a result of being represented as symbols, these weren't expanded by the spread operator. From the MDN docs:

Symbols are [...] hidden from any mechanisms other code will typically use to access the object.

So this was causing the { ...req, params } object to not include all attributes of req!

To include the missing attributes and the params object, a call to Object.assign() was used instead:

const message: ParamsIncomingMessage = Object.assign(req, { params });
this.routes[route][method](message, res);

This Object.assign() method "copies all enumerable own properties", which includes these symbols and no longer requires a type assertion, so the req.headers attribute returns!

Requirements

• I've read and understood the Contributing Guidelines and have done my best effort to follow them.
• I've read and agree to the Code of Conduct.

[codecov]

Codecov Report

Merging #1806 (79e9162) into main (af58002) will increase coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head 79e9162 differs from pull request most recent head 8e19b35. Consider uploading reports for the commit 8e19b35 to get more accurate results

@@            Coverage Diff             @@
##             main    #1806      +/-   ##
==========================================
+ Coverage   82.11%   82.15%   +0.04%     
==========================================
  Files          18       18              
  Lines        1526     1519       -7     
  Branches      442      435       -7     
==========================================
- Hits         1253     1248       -5     
  Misses        175      175              
+ Partials       98       96       -2     

Impacted Files	Coverage Δ
src/receivers/HTTPReceiver.ts	59.03% <100.00%> (+0.21%)	⬆️
src/receivers/SocketModeReceiver.ts	84.61% <100.00%> (-0.50%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[seratch]

Hey @E-Zim, thanks for working on this! Since I am a bit busy for other things right now, I won't be able to review this PR this week. If other assigned reviewers can do within a few days, I will defer to them for PR approvals.

[WilliamBergamin]

Great work 🥇 excellent detailed explanation in the PR totally help me get up to speed on the context

Left a small naming comment for the tests

[WilliamBergamin]

Since this is a test and message is a value we expect and compare against, I think renaming it something more descriptive such as expectedMessage or something similar would provide more context

Let me know what you think

[zimeg]

That is much more clear, nice callout! I have gone and updated all of the tests with this 👍

[WilliamBergamin]

LGTM