Do I understand you correctly that you find it confusing that the catch block is only executed when the call reverted? Would you want the catch block to be executed for a successful call to the function but a failure to decode the return data?

Yes. I think its confusing that the code that is covered syntactically by try/catch causes a revert.
The assumption of a developer is that this is a "safe-zone".
The purpose of try/catch is catch errors caused by things outside of the control of the developer and continue the normal path of the code.
With the current implementation, it is nearly impossible to achieve that, without resorting to low level calls and assembly - since the compiler explicitly generates "revert" calls before calling the external function (to handle "no-contract-at-target") and after the external function returns (when it fails to parse the response)
Note that both of these cases fall under "not under the contact developer's control" category - just like reverts in the called external function

I understand that you can't add a real try/catch, since the EVM only support reverting an external call.
But I DO expect the code-generator to "honor" the developer's intention that reverts inside the try/catch range will be captured.

If you send ether to the called function in the try/catch, how do you distinguish the ether being gone or not in the catch clause?

The try/catch statement was not meant as something like a protection against malicious called contracts, it's rather a way to handle errors that come with reverts. Can you share a bit more about your use-case?

1. the major issue is a revert done by the compiler even before the call (target address is invalid), which obviously doesn't send any funds
2. even in the lesser case (called a function that returns a value, and failed to decode its return value) I think its better to treat it as "exception" and get into the "catch" block.
   If I really expect to transfer value in this call, i'd have to save my balance before the call, and compare the saved balance to current value inside the catch block:
   Yes, its not a nice solution - but that's the edge case.
   Currently, the major case (=target address is mis-configured) goes the wrong path, and require me (the developer) to use a low-level, error-prone "call" API instead of using try/catch block

Can you please share a bit more about your use-case? It is difficult to discuss breaking invariants at such an abstraction level without any examples.

Here is a reference to our code, which is the implementation of "account abstraction" EIP-4337: https://bit.ly/3sMBirB
Some context: account-abstraction is a mechanism that "abstracts" EOA account as a wallet contract. specifically, the signature and nonce validations are done by the wallet itself.
The singleton "EntryPoint" calls the validateUserOp() method on the wallet contract.
Since we attempt to create a "batch" of multiple requests, we need to find out which (if any) causes the entire batch to fail.
For this purpose we wrap the request with try...catch, with the expectation to catch any failure case, and properly handle it (in our case, it is reverting with a custom "FailedOp" structure)

However, the current code is not enough, in case the target address is not a contract, it reverts.
We're very conscious to the gas cost, as this is an overhead of each request that goes through the system.

using address.call is not an option, since its neither safe, nor efficient (cost ~2000 gas more)

The only option we currently have is add an assembly code to do extcodesize(sender) just before calling the method - mimicking the code the compiler inserts when calling the method.

Ok, from your example, I see that the problem is rather the excodesize check and not the decoding failure (as you already said above).
I think changing the behaviour of try/catch with respect to the extcodesize check could be a good change.

I'm less convinced to execute the catch block in case of a successful call and return data decoding. I think we should not change the invariant that if the catch code is executed, the call did not have any effect. Note that ether transfer is not the only state change that can happen and there are state changes that are impossible to detect.

I still think that it should be possible to "catch" (that is, not throw) inside the try...catch block
A possible way is have a maybe a different catch mode, e.g. using s different catch signature. At first I thought of Panic(), but it is not thrown on such cases, so maybe something like:

try func() 
catch DecodeError() {

}

I think that handling of decoding errors is highly required in complex contracts interactions.
Currently, developers are required to use the error-prone (and gas-inefficient) "address.call".

@hrkrshnn noted that if we go to the catch block on excodesizecheck failure, we have to re-add the extcodesizecheck that was recently removed due to a decoding error resulting in a revert anyway #12204

I still think that its worthwhile handling the general case of "void" function: in that case, you already call extcodesize to validate it exists, and it should redirect to the global catch block (if there is one) instead of directly reverting.

Just to chime in here. I have a use case related to ERC165-like "light" interface checking. Basically, try/catch guards could be used to experimentally call an external contract function and act on the basis of which catch block executes. This would have many useful applications. One in particular would be to check if one of the transfer addresses in a token transfer appears to be something that smells like a particular kind of token wallet etc.

Of course, something as simple as:

    function respondsTo(address c, bytes4 sel) internal view returns(bool result) {
        assembly {
            let freeMemory := mload(0x40)
            mstore(freeMemory,sel)
            result := staticcall(gas(),c,freeMemory,4,0,0)
        }
    }

serves such a purpose for view functions that take no parameters. Perhaps Solidity does not want to encourage this kind of usage, but a gas-efficient mechanism that could query external contracts in this way seems like a good thing. I know more efficient dispatchers are being discussed, and this consideration might be one to include in that implementation.

Hello! I often have troubles detecting and catch calls to non contract accounts.

There is one simple example:

  function _getPairName(IUniswapV2Pair pair) 
  internal view returns (string memory) {
    try pair.name{gas : 3000}() returns (string memory name) {
      return name;
    } catch {}
    return _NONAME;
  }

If I face with some non-standart pair without name() method or it is not contract address - all my code reverts while I expect what it should catched in tr..catch block.

@aathan , I have tried use code you provided, but do not understand what ptr is in line
mstore(ptr,sel)

Hello! I often have troubles detecting and catch calls to non contract accounts.

There is one simple example:

  function _getPairName(IUniswapV2Pair pair) 
  internal view returns (string memory) {
    try pair.name{gas : 3000}() returns (string memory name) {
      return name;
    } catch {}
    return _NONAME;
  }

If I face with some non-standart pair without name() method or it is not contract address - all my code reverts while I expect what it should catched in tr..catch block.

I stumbled almost to identical usage problem, where as I need to detect somehow if an address is a deployed / valid UniswapV2Pair or not.
My requirement is to fail "gracefully" and not revert if that address is not a UniswapV2Pair or anything at all.

try catch does not work as expected and reverts where as I need it to catch it and continue on with my non blocking function.

@bogdoslavik Have you figured a way to solve this issue?

Here's my example

address _pair = UniswapV2Library.pairFor(factory, _token0, _token1);

try IUniswapV2Pair(_pair).getReserves() {
  // do stuff if pair exists
} catch {
 // do stuff if pair does not exist
}

This reverts and does not honor the try catch.

It's been a long time since I looked at this but I've corrected ptr in my original comment to read freeMemory, which is what (logically) it was probably meant to say in the first place. This is just building the necessary params for staticcall() to call a function that takes no params (the selector of which you pass to my example code as sel), so that you can inspect the result; I think my assumptino was that your target contract would implement some selector that would allow the boolean true return of staticcall to indicate the selector exists at the target contract.

Read more about staticcall in the EVM docs and/or places like this one: https://nicolabernini.substack.com/p/an-example-staticcall-usage

Perhaps take further discussion to the chat/help groups. Again, this is very old now and I'm just spending a few minutes to answer your question rather than fully confirm everything I've said here and in the prior comment is accurate. Your mileage may vary.

@mkMoSs Before call, you have to check, what calling address have code:

function _isContract(address addr) internal returns (bool) {
  uint size;
  assembly { size := extcodesize(addr) }
  return size > 0;
}

for Solidity v0.8 (fn can be view)

pragma solidity >=0.8.0;

function isContract(address _addr) view returns (bool) {
    return _addr.code.length > 0;
}

But be aware of vulnerability described: stackoverflow

@chriseth : sorry for late response:
there is a side-case that I think its easy to support:
in the
try a.viewCall() returns (xyz) {} catch{}
case:
if the call to viewCall is static, then it surely doesn't modify the state. In this case, it is no brainer that the current contract should not revert trying to parse the return value: it should fail into the "catch" block instead.
I still would like to see a solution for the case where the method is non-static (which requires a new syntax, e.g. catch ParseError {}

@bogdoslavik
Thank you, this is what I ended up doing.

/* 
* UniswapV2Library.pairFor is a pure function and generates the pair address based on factory address and a hex code
* Nothing else can generate this address.
*/
address _pairV2 = UniswapV2Library.pairFor(_factoryV2, _token0, _token1);

// is that pair deployed / existent?
// as of now, there is no other way (that I know of) to check if a pair has been deployed or not
//  try _pairV2.getReserves() catch, reverts which is not what we want
if (_pairV2.code.length > 0) {
    (uint _reserve0, uint _reserve1) = UniswapV2Library.getReserves(_factoryV2, _token0, _token1);

    uint _amountV2 = UniswapV2Library.getAmountIn(_amount, _reserve0, _reserve1);
 // ...  do other stuff
}
// else do other stuff, but do not revert

I did read about the vulnerability issues with this method, as far as I can tell I should have no issues with my particular use case, unless I'm missing something.

Thank you

It's also worth mentioning ERC165

I ran into this today simply because some code i don't control had a try/catch block calling into my contract that happens to have a fallback defined.

The fallback doesn't error and it doesn't return any data, but the caller was expecting an address to be returned if the method they called was successful, so then the decoding process reverted rather than moving into the catch arm.

I'm not 100% sure because I didn't write the code with the try/catch but I'm pretty sure this isn't the behaviour they intended/expected.

For the specific case of a simple return value that doesn't have complex decoding this could be a potential workaround, but the ergonomics degrade rapidly for more complex return types:

(bool success, bytes memory returnData) = foo.staticcall(abi.encodeWithSignature("bar()"));
if (success && returnData.length == 0x20) {
  address result = abi.decode(returnData, (address));
  ...
}
else {
}