Remove streamId from Errors

[smol-ninja]

How do we decide which error should contain streamId?

Some of the errors contain streamId and some of them not. I failed to understand what determines that. IMO we should exclude streamId from errors since they are always part of the function's input data.

RFC @andreivladbrg.

[andreivladbrg]

How do we decide which error should contain streamId?

I think we are reverting with a custom error that includes the ID everywhere when a stream is already created and it is not about a specific unique param (e.g. address to in withdraw).

IMO we should exclude streamId from errors since they are always part of the function's input data

I don't agree with this. I've also mentioned something similar in the past when discussing the "Sablier" prefix.

It is useful to have the ID in the custom error because it improves the DX for integrators with complex systems.

[smol-ninja]

it is not about a specific unique param

Can you please explain with an example? Its not clear to me what you meant here.

streamId is missing in the following errors:

1. SablierFlow_BrokerAddressZero
2. SablierFlow_TransferAmountZero
3. SablierFlow_RatePerSecondZero
4. SablierFlow_RefundAmountZero
5. SablierFlow_SenderZeroAddress
6. SablierFlow_WithdrawToZeroAddress

And its included in the following:

1. SablierFlow_Overrefund

etc.

[andreivladbrg]

Can you please explain with an example? Its not clear to me what you meant here.

Sure, I will try to be more specific. I mean, input parameters are checked against something other than internal storage or amounts calculated for that ID.

All these: SablierFlow_BrokerAddressZero, SablierFlow_TransferAmountZero, SablierFlow_RatePerSecondZero, SablierFlow_RefundAmountZero, SablierFlow_SenderZeroAddress, and SablierFlow_WithdrawToZeroAddress are reverted due to an input parameter checked against something different than the storage. For example, in the case of withdraw, to it is checked against address(0), which has nothing to do with information that can be found for a specific ID.

SablierFlow_SenderZeroAddress is used in the create function, so we don't have an ID yet for the stream (as I mentioned in my previous message already created)

---

Lmk if I made myself clear, and if you agree.

[smol-ninja]

Thank you for the explanation. I now understand the purpose of using input params in error msgs. I also agree with you that excluding streamId would be a bad idea. But then, why don't we include it in all errors such as SablierFlow_RatePerSecondZero, SablierFlow_WithdrawToZeroAddress etc?

It is useful to have the ID in the custom error because it improves the DX for integrators with complex systems.

So, imagine an integrator writes a function that withdraws from multiple streams and some of them causes SablierFlow_WithdrawToZeroAddress error. Without streamId included in the error msg, it would be difficult to identify from the logs which stream ID caused the revert.

[andreivladbrg]

Without streamId included in the error msg, it would be difficult to identify from the logs which stream ID caused the revert

fair enough, let's include streamId in each error

[andreivladbrg]

#177