search

zonemaster / zonemaster-backend

The Zonemaster Backend - part of the Zonemaster project
Other
14 stars 23 forks source link

RPCAPI: call Translator’s instance() method, instead of new() #1167

Closed marc-vanderwal closed 5 months ago

marc-vanderwal commented 6 months ago

Purpose

This PR fixes a bug causing the RPCAPI to only work once, before always returning HTTP 500 errors until the daemon is restarted.

Context

When RPCAPI needs a Translator, it called the latter module’s new() method, instead of instance().

It’s a singleton object, so calling a method named new() makes no sense. It is also marked as deprecated in the documentation. Besides, after a recent refactoring of Zonemaster::Engine::Translator, new() is no longer idempotent: calling it repeatedly will crash the program. And indeed, calling the get_test_results RPCAPI method only worked once, then always returned an error until the RPCAPI daemon is restarted.

Note: this change requires merging of https://github.com/zonemaster/zonemaster-engine/pull/1347 in Engine.

Changes

How to test this PR

Prerequisites: PR https://github.com/zonemaster/zonemaster-engine/pull/1347 needs to be merged first.

Run the unit test suite, or at least t/test01.t; expect no errors. One of the symptoms of the problem being fixed by this PR is that the t/test01.t test script did not pass.

Get a hold of a test ID from a previous domain test in the database, or run a dummy test (e.g. zmtest localhost).

Then, run zmb get_test_results --test-id $TEST_ID --lang $LANGUAGE as many times as there are RPCAPI workers (by default, 5), where $TEST_ID is the ID of any previous test and $LANGUAGE is any supported language. Then, run the same command a few times more and expect no errors on output.

MichaelTimbert commented 4 months ago

This PR works as expected on Ubuntu 20.04.6 and 22.04.4. For testing i have run several test with zmtest on different domain and fetched successfully with zmb get_test_results --test-id $TEST_ID --lang $LANGUAGE. I have tested with multiple combination of Domains/Language, Each time I receive the json result.

t/test01.t PASS on both version of Ubuntu

zonemaster-backend$ PERL_DL_NONLAZY=1 "/usr/bin/perl" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'inc', 'blib/lib', 'blib/arch')" t/test01.t
t/test01.t .. # not recording
t/test01.t .. 1/?     # running the agent on test 8cf7cadaea721bf4
t/test01.t .. 2/? # running the agent on test 1cac592b8e3f891d
        # running the agent on test ad163453bdcca2b0
t/test01.t .. 5/?     # running the agent on test c86ca000f14d0ac9
    # running the agent on test e6edee65f6e841c5
    # running the agent on test ebd94e37507a404d
t/test01.t .. ok
All tests successful.
Files=1, Tests=8, 29 wallclock secs ( 0.02 usr  0.00 sys +  2.03 cusr  0.10 csys =  2.15 CPU)
Result: PASS

I don't think it's related to this PR but if I run all unit tests I get a translation error ONLY in Ubuntu 22.04.4.

t/translator.t ............. 1/?
#   Failed test 'Translating a backend-specific TEST_DIED message tag works'
#   at t/translator.t line 82.
#                   'An error occured and Zonemaster could not start or finish the test.'
#     doesn't match '(?^u:\AUne erreur est survenue )'
# Looks like you failed 1 test of 9.
t/translator.t ............. Dubious, test returned 1 (wstat 256, 0x100)