Problem/Motivation

Mapillary Logs (mapillary_logs) has a "Log errors" setting that is on by default, but with the default settings nothing is logged at all:

  • MapillaryLogsService::logApiCall() returns early when log_api_calls is off, and the log_errors check only runs after that. Error logging therefore only works when every API call is logged too. The defaults are log_api_calls: false and log_errors: true, so a failed API request is never logged by this module on a fresh install.
  • The messages build the endpoint, entity type, entity ID, image ID and container ID into the message string ("Mapillary API call to {$endpoint} failed ...") instead of passing them as placeholders. Every call produces a different message, so /admin/reports/dblog cannot group them and the messages cannot be translated.
  • MapillaryLogsHooks only logs entity operations for a hardcoded list of mapillary_image, mapillary_sequence and mapillary_organization. Map features (mapillary_map_feature) and detections (mapillary_detection) are not logged.
  • The description of "Log errors" promises "missing configuration, and import exceptions", but only failed API requests reach it. The description of "Log entity CRUD" lists only image, sequence and organization.
  • The logger.channel.mapillary_logs service is defined but never used; the service gets its channel from logger.factory.
  • mapillary_logs.info.yml requires dblog and mapillary_entity. The module writes to logger.factory, so any logger (syslog, a log shipper) works, and it only compares entity type IDs, so it does not need mapillary_entity.
  • README.md and AGENTS.md list an administer mapillary_logs permission. The module has no permissions.yml; the settings route uses administer mapillary.

Steps to reproduce

  1. Enable mapillary_logs and dblog, and leave the logging settings at their defaults.
  2. Set an invalid token: drush cset mapillary.settings access_token 'MLY|1|invalid'.
  3. Import any image, for example through a Mapillary image field on a node.
  4. Open /admin/reports/dblog and filter by type mapillary_logs. There is no entry for the failed request.

Proposed resolution

  • Check log_errors on its own for failed calls, and log_api_calls only for successful ones.
  • Pass every variable part of the messages as a placeholder.
  • Log entity operations for every content entity type provided by a Mapillary module, instead of a hardcoded list, so map features, detections and future entity types are included.
  • Correct the "Log errors" and "Log entity CRUD" descriptions.
  • Remove the unused logger.channel.mapillary_logs service.
  • Remove the dblog and mapillary_entity dependencies. mapillary_field stays for now, because the API and viewer event classes live there.
  • Remove the nonexistent permission from README.md and AGENTS.md.

MapillaryApiClient also writes failed requests to its own mapillary_field channel, so with this fix an error is logged twice when Mapillary Logs is enabled. That, the missing token (no event is dispatched) and the log level of expected errors such as 404 belong to the API client and are left to a follow-up that moves the client to the base module.

Remaining tasks

  • Tests: an error is logged with log_api_calls off and log_errors on; no error is logged with log_errors off; messages use placeholders; map feature and detection operations are logged; config entities such as bundle types are not.
  • Update MapillaryLogsCrudTest, which searches the message text and needs to search the variables instead.
  • CHANGELOG: on a default install, failed API requests are now logged.

User interface changes

  • Corrected descriptions for "Log errors" and "Log entity CRUD".

API changes

  • None. MapillaryLogsService and MapillaryLogsHooks are final, and their public methods keep their signatures.
  • The logger.channel.mapillary_logs service is removed. Code that uses logger.factory with the mapillary_logs channel still works.

Data model changes

  • None. Removing a dependency does not uninstall dblog or mapillary_entity on existing sites.

Issue fork mapillary-3629730

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

orkutmuratyilmaz created an issue. See original summary.

  • 1f7dc196 committed on 1.0.x
    bug: #3629730: Mapillary Logs: error logging only works when API call...
orkutmuratyilmaz’s picture

Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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