mirror of
				https://github.com/zulip/zulip.git
				synced 2025-10-31 20:13:46 +00:00 
			
		
		
		
	management: Don't silence send_server_data_to_push_bouncer exceptions.
When these exceptions are thrown from the request-to-bouncer functions inside of manage.py register_server/update_analytics_counts, they shouldn't be silenced, merely calling maybe_mark_pushes_disabled in the background. This results in the occurrence of the error not being shown to the user. Failure to upload analytics data when running these commands should result in a loud, obvious error. Failure of running register_server before this change: ``` ./manage.py register_server This command registers your server for the Mobile Push Notifications Service. Doing so will share basic metadata with the service's maintainers: * This server's configured hostname: zulipdev.com:9991 * This server's configured contact email address: desdemona+admin@zulip.com * Metadata about each organization hosted by the server; see: <https://zulip.com/doc-permalinks/basic-metadata> Use of this service is governed by the Zulip Terms of Service: <https://zulip.com/policies/terms> Do you want to agree to the Zulip Terms of Service and proceed? [Y/n] Mobile Push Notification Service registration successfully updated! ``` The occurrence of the error is not revealed to the user. Same concern applies to the update_analytics_counts command. After this change: ``` ./manage.py register_server This command registers your server for the Mobile Push Notifications Service. Doing so will share basic metadata with the service's maintainers: <...> Do you want to agree to the Zulip Terms of Service and proceed? [Y/n] Traceback (most recent call last): File "/srv/zulip/./manage.py", line 150, in <module> execute_from_command_line(sys.argv) File "/srv/zulip/./manage.py", line 115, in execute_from_command_line utility.execute() File "/srv/zulip-venv-cache/bb36fc1fcb6d8c70a9a0bcb7bac45d78623a9ff4/zulip-py3-venv/lib/python3.10/site-packages/django/core/management/__init__.py", line 436, in execute self.fetch_command(subcommand).run_from_argv(self.argv) File "/srv/zulip-venv-cache/bb36fc1fcb6d8c70a9a0bcb7bac45d78623a9ff4/zulip-py3-venv/lib/python3.10/site-packages/django/core/management/base.py", line 413, in run_from_argv self.execute(*args, **cmd_options) File "/srv/zulip/zerver/lib/management.py", line 97, in execute super().execute(*args, **options) File "/srv/zulip-venv-cache/bb36fc1fcb6d8c70a9a0bcb7bac45d78623a9ff4/zulip-py3-venv/lib/python3.10/site-packages/django/core/management/base.py", line 459, in execute output = self.handle(*args, **options) File "/srv/zulip/zerver/management/commands/register_server.py", line 137, in handle send_server_data_to_push_bouncer(consider_usage_statistics=False, raise_on_error=True) File "/srv/zulip/zerver/lib/remote_server.py", line 453, in send_server_data_to_push_bouncer response = send_to_push_bouncer( File "/srv/zulip/zerver/lib/remote_server.py", line 233, in send_to_push_bouncer raise JsonableError(msg) zerver.lib.exceptions.JsonableError: Duplicate registration detected. ```
This commit is contained in:
		
				
					committed by
					
						 Tim Abbott
						Tim Abbott
					
				
			
			
				
	
			
			
			
						parent
						
							1532b87910
						
					
				
				
					commit
					517538a296
				
			| @@ -95,4 +95,4 @@ class Command(ZulipBaseCommand): | |||||||
|             logger.info("Sleeping %d seconds before reporting...", delay) |             logger.info("Sleeping %d seconds before reporting...", delay) | ||||||
|             time.sleep(delay) |             time.sleep(delay) | ||||||
|  |  | ||||||
|             send_server_data_to_push_bouncer(consider_usage_statistics=True) |             send_server_data_to_push_bouncer(consider_usage_statistics=True, raise_on_error=True) | ||||||
|   | |||||||
| @@ -387,13 +387,17 @@ def should_send_analytics_data() -> bool:  # nocoverage | |||||||
|     return settings.ANALYTICS_DATA_UPLOAD_LEVEL > AnalyticsDataUploadLevel.NONE |     return settings.ANALYTICS_DATA_UPLOAD_LEVEL > AnalyticsDataUploadLevel.NONE | ||||||
|  |  | ||||||
|  |  | ||||||
| def send_server_data_to_push_bouncer(consider_usage_statistics: bool = True) -> None: | def send_server_data_to_push_bouncer( | ||||||
|  |     consider_usage_statistics: bool = True, raise_on_error: bool = False | ||||||
|  | ) -> None: | ||||||
|     logger = logging.getLogger("zulip.analytics") |     logger = logging.getLogger("zulip.analytics") | ||||||
|     # first, check what's latest |     # first, check what's latest | ||||||
|     try: |     try: | ||||||
|         result = send_to_push_bouncer("GET", "server/analytics/status", {}) |         result = send_to_push_bouncer("GET", "server/analytics/status", {}) | ||||||
|     except (JsonableError, orjson.JSONDecodeError) as e: |     except (JsonableError, orjson.JSONDecodeError) as e: | ||||||
|         maybe_mark_pushes_disabled(e, logger) |         maybe_mark_pushes_disabled(e, logger) | ||||||
|  |         if raise_on_error:  # nocoverage | ||||||
|  |             raise | ||||||
|         return |         return | ||||||
|  |  | ||||||
|     # Gather only entries with IDs greater than the last ID received by the push bouncer. |     # Gather only entries with IDs greater than the last ID received by the push bouncer. | ||||||
| @@ -452,6 +456,8 @@ def send_server_data_to_push_bouncer(consider_usage_statistics: bool = True) -> | |||||||
|             "POST", "server/analytics", request.model_dump(round_trip=True) |             "POST", "server/analytics", request.model_dump(round_trip=True) | ||||||
|         ) |         ) | ||||||
|     except (JsonableError, orjson.JSONDecodeError) as e: |     except (JsonableError, orjson.JSONDecodeError) as e: | ||||||
|  |         if raise_on_error:  # nocoverage | ||||||
|  |             raise | ||||||
|         maybe_mark_pushes_disabled(e, logger) |         maybe_mark_pushes_disabled(e, logger) | ||||||
|         return |         return | ||||||
|  |  | ||||||
|   | |||||||
| @@ -134,7 +134,7 @@ that registration and saving the updated secret in | |||||||
|             "/api/v1/remotes/server/register", request |             "/api/v1/remotes/server/register", request | ||||||
|         ) |         ) | ||||||
|  |  | ||||||
|         send_server_data_to_push_bouncer(consider_usage_statistics=False) |         send_server_data_to_push_bouncer(consider_usage_statistics=False, raise_on_error=True) | ||||||
|  |  | ||||||
|         if response.json()["created"]: |         if response.json()["created"]: | ||||||
|             print( |             print( | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user