php-jwt icon indicating copy to clipboard operation
php-jwt copied to clipboard

Add code for each Exception

Open nicumicle opened this issue 3 years ago • 5 comments

Adding a code for each exception will allow us to translate the exception messages and also offer a simple way to find what exception is thrown.

Issue-455

nicumicle avatar Sep 21 '22 06:09 nicumicle

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Sep 21 '22 06:09 google-cla[bot]

It would be even better if the exception codes are 'unique' within the php ecosystem: When translating exceptions, consumers probably use a global exception handler to catch things and global exception code uniqueness could help here - when multiple different libs don't throw "1" as exception code.

One approach is to use timestamp of "now" (timestamp when the exception is added, easy to generate with eg. date +%s on shell, or via PHP). This makes it 'unique' with a pretty high probability, when other libs use the same strategy.

The int itself isn't important as such since you have the class constants in ExceptionCodes already, there is no good reason to enumerate them?

What do you think?

lolli42 avatar Oct 08 '22 12:10 lolli42

Use case of unique exception codes: We add a unique code in our project whenever we throw an exception (but we don't have this ExceptionCodes constant resolver) and we actively test each code is only used once. In develop-mode, we then link to the projects documentation (or a wiki) and use the exception code as path part. People can add hints there how to solve an exception and when or why they typically appear.

lolli42 avatar Oct 08 '22 12:10 lolli42

It would be even better if the exception codes are 'unique' within the php ecosystem: When translating exceptions, consumers probably use a global exception handler to catch things and global exception code uniqueness could help here - when multiple different libs don't throw "1" as exception code.

One approach is to use timestamp of "now" (timestamp when the exception is added, easy to generate with eg. date +%s on shell, or via PHP). This makes it 'unique' with a pretty high probability, when other libs use the same strategy.

The int itself isn't important as such since you have the class constants in ExceptionCodes already, there is no good reason to enumerate them?

What do you think? @loli42,

I don't see this as a good approach.

In my opinion, this package should not care about how other apps will use the codes. This package should only make sure that it will always return a unique code ID for each error.

So, for example, I'm planning to use these error codes, and create a wrapper, that will translate the exceptions in case I know the error code.


try{
	  try{
	     JWT::decode($jwt, new Key($key, 'HS256'));
	  } catch(Exception $e){
	     $this->handleException($e);
	  }	 
}catch(Exception $e){

   return new JsonReponse(['error' => $e->getMessage(),'code' => 'unable.to.decode'], 400);
}

return new JSonReponse(['success' => true], 200);
  

And, I would implement handleException like this:


	private function handleException(Exception $e)
	{
	    switch($e->getCode() {
	    
	         case ExceptionCodes::KEY_NOT_EMPTY:
	         	$message = Translations::translate('Some translated string');
	         	break;
	         ........
	         
	         default:
	         	$message = $e->getMesasge();
	    }
	    
	    throw new Exception($message, $e->getCode(), $e);
	}

So, if at some point, the exception codes of the library will be changed, I will not need to do anything on my code.

Also, with this example, you can convert these error codes to be unique in your app. For example, for JWT, I want to have error codes from 2000. From the previous example, you just need to add the 2000 to the newly generated exception:

	throw new Exception($message, 2000 + $e->getCode(), $e);

nicumicle avatar Oct 11 '22 02:10 nicumicle

I get why you want codes in a reasonable range when using them with your approach. Of course one should have try blocks around usages of a library and then translate exceptions to something that suites the own ecosystem.

lolli42 avatar Oct 11 '22 05:10 lolli42