Refactor middleware processing (v2)

[aoberoi]

Summary

This PR mainly resolves two issues:

1. When an error occurs in multiple listener middleware chains, then an aggregate error should flow up through the global middleware chain (and the global error handler). Previously, the first listener middleware chain to reject would flow through the global middleware chain, resulting in other potential rejections being lost.
2. The word "context" and abbreviation "ctx" was used in several places to refer to the whole set of middleware or listener arguments. This was confusing because there is a middleware argument called context. The variables and types that used context to refer to the set of listener arguments have been removed/renamed.

• Implement
• Get linting to pass
• Fix tests
• Clean up / remove dead code
• Other related cleanup

Requirements (place an x in each [ ])

• 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.

[aoberoi]

There's a few things that I think could be improved in the future coming out of this PR. The biggest one is the fact that the Middleware<Args> type includes an optional property: next?: NextFn;.

The problem with this is for TypeScript users who are writing middleware (global or listener), they will have to conditionally check the existence of the next argument before using it. This is unfortunate because for middleware the next argument is always present. Internally, we use the non-null assertion operator (!) to convince TypeScript that the argument is present (e.g. await next!();). But this isn't a technique we should be advertising to users.

It would be ideal if in the future we could separate the Middleware<Args> type from the Listener<Args> type, where the former always has the next argument and the latter never has it. During the course of building this change, we tried to get to this ideal. Since many of the App method signatures are variadic (e. g. app.event(eventType, ...middlewareAndListener)) we're not able to separate these two types in a way that preserves the developer experience. Ideally, TypeScript could describe a function signature where the last parameter in rest parameters could have a different type. There might be a way to do this, but exploring the options takes a significant amount of time and we wanted to get this release in users' hands. We should come back to this later.

[aoberoi]

i went down a massive rabbit hole trying to get this to work with regular ES Module import ... from ... syntax. I didn't leave a trail of where this led me in the code, but if anyone comes back looking for an explanation here it is:

there's an issue with a difference between the types and the runtime. its described in detail here: es-shims/Promise.allSettled#5. the conclusion is that the rumtime behavior is spec-compliant, but typescript's output for compiling (downleveling) the import and calls are wrong. an issue was created for that: microsoft/TypeScript#35420.

the workaround suggested also did not work. a different error occurred (lost the exact error message but its not too hard to recreate if you need it). in the end i decided that the quickest fix here would be to just use the uglier, non-standard, less consistent import syntax.

hopefully when we update to a minimum node version of >= 12.9.0 or when we upgrade our minimum TypeScript version (which we don't declare right now) to one where the bug is fixed, this problem will just go away.

[codecov]

Codecov Report

Merging #440 into @slack/bolt@next will increase coverage by 0.34%.
The diff coverage is 98.00%.

@@                 Coverage Diff                  @@
##           @slack/bolt@next     #440      +/-   ##
====================================================
+ Coverage             85.71%   86.05%   +0.34%     
====================================================
  Files                     7        7              
  Lines                   532      538       +6     
  Branches                156      157       +1     
====================================================
+ Hits                    456      463       +7     
  Misses                   48       48              
+ Partials                 28       27       -1     

Impacted Files	Coverage Δ
src/App.ts	89.20% <95.23%> (+0.79%)	⬆️
src/conversation-store.ts	96.29% <100.00%> (ø)
src/errors.ts	100.00% <100.00%> (ø)
src/middleware/builtin.ts	82.78% <100.00%> (ø)
src/middleware/process.ts	100.00% <100.00%> (ø)

---

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 ffb48ee...edfcfbc. Read the comment docs.

[aoberoi]

I can make a couple changes to improve coverage.

[aoberoi]

yussss, finally increased coverage.

[seratch]

I've played around with the next! issue a bit and realized that the only way to improve it is, as @aoberoi mentioned, to have different types for listeners and middleware.

For your reference: here is my branch that works.
seratch@1225079