Repository navigation
Error in catch clause should be unknown #26174
Description
Search Terms
catch unknown type
Suggestion
Today I had another situation where I would have wanted #13219, but a less complex feature that would have prevented the bug as well would have been if the error in catch clauses had the new unknown type instead of any, forcing me to narrow it down before accessing properties.
Ideally this would be the same for the Promise rejection type.
Use Cases
Making error handling more type safe. Currently any prevents type narrowing.
Examples
This is a real world example of a bug:
let pipeline: Pipeline
try {
;({ body: pipeline } = await buildkiteClient.post('organizations/sourcegraph/pipelines', {
body: buildkitePipeline,
json: true,
}))
} catch (err) {
if (
err.error &&
Array.isArray(err.error.errors) &&
err.errors[0] &&
err.errors[0].field === 'name' &&
err.errors[0].code === 'already_exists'
) {
console.log(`Buildkite pipeline ${repoName} already exists, skipping creation`)
pipeline = await buildkiteClient.get(`organizations/sourcegraph/pipelines/${repoName}`)
} else {
throw err
}
}Very easy to miss in code review - it should have been err.error.errors. This is an error returned by a real API.
Checklist
My suggestion meets these guidelines:
- This wouldn't be a breaking change in existing TypeScript / JavaScript code (with a flag)
- This wouldn't change the runtime behavior of existing JavaScript code
- This could be implemented without emitting different JS based on the types of the expressions
- This isn't a runtime feature (e.g. new expression-level syntax)
Activity
mhegazy commentedon Aug 3, 2018
This is only possible if #25720, #10715, #25172, and #21732 are implemented. otherwise it is a massive breaking change.
donaldpipowitch commentedon Oct 28, 2018
I'd love to see this as well. I just made a PR to get this for Reacts
componentDidCatch(DefinitelyTyped/DefinitelyTyped#30068), because it would have prevented a real world bug in our code and looked for an equivalent issue for the nativetry/catch.falsandtru commentedon Jan 3, 2020
I want to enable this change by a compiler option. Or want to declare only unknown type like
catch (reason: unknow) { }.gerardolima commentedon May 5, 2021
I believe the default inferred type for
catch (err) { }should beunknown, which it is, instead ofanywhich is unsafe and highly discouraged.unknownis safer and provides better semantics; I really don't understand why this is not the default for tsc.please see: typescript-eslint/typescript-eslint#3348
wbt commentedon Dec 17, 2021
It appears this has been implemented in #41013 and should likely be closed.
https://devblogs.microsoft.com/typescript/announcing-typescript-4-4-beta/#use-unknown-catch-variables
docs: real-world bugs this design would have prevented