Adding proper exception handling to match with the legacy implementation

Author: Hrithik98

setup.py

@@ -27,6 +27,7 @@
         "six>=1.15.0",
         "urllib3>=1.26.2,<2",
         "legacy-cgi>=2.6.2; python_version>='3.10'",
+        "google-api-python-client"
     ],
     classifiers=[
         "Programming Language :: Python :: 3",

src/google/appengine/api/modules/modules.py

@@ -294,7 +294,7 @@ def get_versions(module=None):
     response = request.execute()
   except errors.HttpError as e:
     if e.resp.status == 404:
-      raise InvalidModuleError(f"Module '{module}' not found.") from e
+      raise InvalidModuleError(f"") from e
     _raise_error(e)
 
   return [version['id'] for version in response.get('versions', [])]
@@ -348,7 +348,7 @@ def get_default_version(module=None):
     response = request.execute()
   except errors.HttpError as e:
     if e.resp.status == 404:
-        raise InvalidModuleError(f"Module '{module}' not found.") from e
+        raise InvalidModuleError(f"") from e
     _raise_error(e)
 
   allocations = response.get('split', {}).get('allocations')
@@ -430,12 +430,15 @@ def get_num_instances(
   try:
     response = request.execute()
   except errors.HttpError as e:
+    if e.resp.status == 404:
+        raise InvalidModuleError(f"") from e
     _raise_error(e)
 
-  if 'manualScaling' in response:
-      return response['manualScaling'].get('instances')
+  if 'manualScaling' not in response:
+      raise InvalidVersionError(f"")
+  
+  return response['manualScaling'].get('instances')
 
-  return 0
 
 def get_num_instances_legacy(module, version):
   def _ResultHook(rpc):
@@ -753,6 +756,8 @@ def get_hostname(
   except errors.HttpError as e:
     _raise_error(e)
 
+  if req_module not in services:
+    raise InvalidModuleError(f"")
   # Legacy Applications (Without "Engines")
   if len(services) == 1 and services[0] == 'default':
     if req_module != 'default':
@@ -832,4 +837,3 @@ def _ResultHook(rpc):
   response = modules_service_pb2.GetHostnameResponse()
   return _MakeAsyncCall('GetHostname', request, response,
                         _ResultHook).get_result()
-

tests/google/appengine/api/modules/modules_test.py

@@ -227,7 +227,7 @@ def testGetVersions_InvalidModule(self):
     mock_request.execute().AndRaise(self._CreateHttpError(404))
     self.mox.ReplayAll()
     with self.assertRaisesRegex(modules.InvalidModuleError,
-                                  "Module 'foo' not found."):
+                                  ""):
       modules.get_versions(module='foo')
 
   # --- Tests for Legacy get_versions ---
@@ -366,7 +366,7 @@ def testGetDefaultVersion_InvalidModule(self):
     self.mox.ReplayAll()
 
     with self.assertRaisesRegex(modules.InvalidModuleError,
-                                  "Module 'foo' not found."):
+                                  ""):
       modules.get_default_version(module='foo')
 
   # --- Tests for legacy get_default_version ---
@@ -456,7 +456,9 @@ def testGetNumInstances_NoManualScaling(self):
                       versionsId='v1').AndReturn(mock_request)
     mock_request.execute().AndReturn({'automaticScaling': {}})
     self.mox.ReplayAll()
-    self.assertEqual(0, modules.get_num_instances())
+
+    with self.assertRaises(modules.InvalidVersionError):
+      modules.get_num_instances()
 
   def testGetNumInstances_InvalidVersion(self):
     os.environ['MODULES_USE_ADMIN_API'] = 'true'
@@ -479,7 +481,7 @@ def testGetNumInstances_InvalidVersion(self):
                       versionsId='v-bad').AndReturn(mock_request)
     mock_request.execute().AndRaise(self._CreateHttpError(404))
     self.mox.ReplayAll()
-    with self.assertRaises(modules.InvalidVersionError):
+    with self.assertRaises(modules.InvalidModuleError):
       modules.get_num_instances(version='v-bad')
 
   # --- Tests for updated get_num_instances ---
@@ -995,7 +997,7 @@ def testGetHostname_WithVersion_NoInstance(self):
     self.mox.StubOutWithMock(modules, '_get_project_id')
     modules._get_project_id().AndReturn('project')
     self.mox.StubOutWithMock(modules, 'get_modules')
-    modules.get_modules().AndReturn(['default', 'other'])
+    modules.get_modules().AndReturn(['default', 'other', 'foo'])
     mock_apps = self.mox.CreateMockAnything()
     mock_get_request = self.mox.CreateMockAnything()
     mock_admin_api_client.apps().AndReturn(mock_apps)
@@ -1011,14 +1013,12 @@ def testGetHostname_Instance_Success(self):
     mock_client_1 = self.mox.CreateMockAnything()
     mock_client_2 = self.mox.CreateMockAnything()
 
-    # Mock the two main dependencies of get_hostname
     self.mox.StubOutWithMock(modules, '_get_admin_api_client_with_useragent')
     modules._get_admin_api_client_with_useragent(
         'get_hostname').AndReturn(mock_client_1)
     self.mox.StubOutWithMock(modules.discovery, 'build')
     modules.discovery.build('appengine', 'v1').AndReturn(mock_client_2)
 
-    # Mock the helper functions
     self.mox.StubOutWithMock(modules, '_get_project_id')
     modules._get_project_id().AndReturn('project')
     self.mox.StubOutWithMock(modules, 'get_modules')
@@ -1028,15 +1028,13 @@ def testGetHostname_Instance_Success(self):
     self.mox.StubOutWithMock(modules, 'get_current_version_name')
     modules.get_current_version_name().AndReturn('v1')
 
-    # Set up expectations for the first client call
     mock_apps_1 = self.mox.CreateMockAnything()
     mock_get_request = self.mox.CreateMockAnything()
     mock_client_1.apps().AndReturn(mock_apps_1)
     mock_apps_1.get(appsId='project').AndReturn(mock_get_request)
     mock_get_request.execute().AndReturn(
         {'defaultHostname': 'project.appspot.com'})
 
-    # Set up expectations for the second client call
     mock_apps_2 = self.mox.CreateMockAnything()
     mock_services_2 = self.mox.CreateMockAnything()
     mock_versions_2 = self.mox.CreateMockAnything()