While working on REST module issues relating to error responses (and hence exception subscribers), I noticed this:

  exception.default_json:
    class: Drupal\Core\EventSubscriber\ExceptionJsonSubscriber
    tags:
      - { name: event_subscriber }
…
  exception.custom_page_json:
    class: Drupal\Core\EventSubscriber\ExceptionJsonSubscriber
    tags:
      - { name: event_subscriber }
    arguments: ['@config.factory', '@path.alias_manager', '@http_kernel']

i.e. two services using the same class, with different arguments. The second one is never used, and has nonsensical arguments.

We can remove it.

CommentFileSizeAuthor
#2 2833469-2.patch665 bytesWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Status: Active » Needs review
FileSize
665 bytes
Wim Leers’s picture

tedbow’s picture

Status: Needs review » Reviewed & tested by the community

I double checked that the string literal 'exception.custom_page_json' not found anywhere else in core except once in core.services.yml.
So the service is never used. RTBC!

  • alexpott committed 1c762d1 on 8.3.x
    Issue #2833469 by Wim Leers: Remove 'exception.custom_page_json' service...
alexpott’s picture

Version: 8.3.x-dev » 8.2.x-dev
Category: Task » Bug report

Well it is not doing nothing... it means for every json exception we are firing \Drupal\Core\EventSubscriber\ExceptionJsonSubscriber::on400() etc twice :) to me this is a bug and we should backport this to 8.2.x since event listeners are not API and we're not stopping the thing from running. Nice find.

Committed 1c762d1 and pushed to 8.3.x. Thanks!

Leaving at rtbc aginst 8.2.x to discuss with the release managers.

alexpott’s picture

Version: 8.2.x-dev » 8.3.x-dev
Category: Bug report » Task
Status: Reviewed & tested by the community » Fixed

Discussed with @catch he pointed out that whilst code was indeed running twice - there's no bug caused by this and no real performance impact - so moving back to 8.3.x and setting fixed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.