feat: adding storage client #191
Conversation
pgautier404
force-pushed
the
add-storage-client
branch
from
0929090 to
d4c5255
Compare
pgautier404
requested review from
rishtigupta,
nand4011,
cprice404,
malandis and
bruuuuuuuce
pgautier404
force-pushed
the
add-storage-client
branch
from
e1a078e to
1237012
Compare
| /** | ||
| * Item with specified name doesn't exist | ||
| */ | ||
| public const ITEM_NOT_FOUND_ERROR = "ITEM_NOT_FOUND_ERROR"; |
There was a problem hiding this comment.
We should call this STORE_ITEM... to disambiguate vs cache items.
There was a problem hiding this comment.
There is no not found error for cache items-- those are represented as misses. This is also how this is defined in the JS SDK. I'm not necessarily opposed to changing this error code to be specific to storage for future-proofing, but we should be consistent across SDKs. I'm going to leave this for now to maintain consistency with other SDKs but am happy to discuss further.
| /** | ||
| * Item with specified name doesn't exist | ||
| */ | ||
| class ItemNotFoundError extends SdkError | ||
| { | ||
| public string $errorCode = MomentoErrorCode::ITEM_NOT_FOUND_ERROR; | ||
| public string $messageWrapper = 'An item with the specified name does not exist. To resolve this error, make sure you have created the item before attempting to use it'; | ||
| } | ||
|
|
There was a problem hiding this comment.
Rename should extend here too.
| public function type(): string | ||
| { | ||
| throw $this->innerException(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Are we throwing exceptions here vs returning null?
There was a problem hiding this comment.
Good question, I wasn't sure how far to take this. Same question applies to found() as well, which makes sense as both null and false in the error class.
There was a problem hiding this comment.
I switched it so both type() and found() return null from the error class.
No description provided.