Make safelyExecute type safe

[bergeron]

Explanation

safelyExecute and safelyExecuteWithTimeout are convenient utility functions, but they were returning any which removes type safety. This PR makes them generic so they return the type of the underlying operation.

References

Changelog

@metamask/controller-utils

• BREAKING: Make safelyExecute and safelyExecuteWithTimeout generic so they preserve types

Checklist

• I've updated the test suite for new or updated code as appropriate
• I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
• I've highlighted breaking changes using the "BREAKING" category above as appropriate

[mcmire]

Thanks, this makes sense! Could you mark this as a breaking change for both safelyExecute and safelyExecuteWithTimeout in the PR description? I've also left a couple of comments and a question.

[mcmire]

This looks good to me!

[Gudahtt]

@bergeron How would you describe the impact of these changes? It looks like they'd have a functional impact.

This suggests that there may have been cases where blockchainMetadata was nullish, which previously would have thrown an error on this line. But now it's no longer thrown.

Similarly, below in getNftContractInformation, the changes suggest that address and contract.name would previously be missing sometimes, but now they're always included.

Is that the case? Or are the affected scenarios really non-existent, with these changes being made just for type reasons.

[Gudahtt]

It looks like this does fix something. In the case where the request fails, this would crash on this line previously, but now it doesn't.

Too bad this isn't represented in tests either, there are no tests for this method (as it's private). And even indirect uses of it are stubbed out in the tests 🤦