Bug 37546 - We should output error messages alongside error codes for z39.50 errors
Summary: We should output error messages alongside error codes for z39.50 errors
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Martin Renvoize (ashimema)
QA Contact: David Cook
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-08-01 15:34 UTC by Martin Renvoize (ashimema)
Modified: 2025-01-14 05:01 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 37546: Output error message from ZOOM in Z3950 errors (2.44 KB, patch)
2024-08-01 15:38 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Example error message (21.26 KB, image/png)
2025-01-13 00:10 UTC, David Cook
Details
Bug 37546: Output error message from ZOOM in Z3950 errors (2.44 KB, patch)
2025-01-13 07:52 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37546: Error message output improvements (1.92 KB, patch)
2025-01-13 07:52 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37546: Output error message from ZOOM in Z3950 errors (2.49 KB, patch)
2025-01-14 04:58 UTC, David Cook
Details | Diff | Splinter Review
Bug 37546: Error message output improvements (1.98 KB, patch)
2025-01-14 04:58 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2024-08-01 15:34:06 UTC
When performing a z39.50 search during cataloging, there are various errors that can be returned from ZOOM.. these have errors codes, but they also have nicer strings too.. which we're hiding!

We should return both in the result.
Comment 1 Martin Renvoize (ashimema) 2024-08-01 15:38:33 UTC
Created attachment 169953 [details] [review]
Bug 37546: Output error message from ZOOM in Z3950 errors
Comment 2 Owen Leonard 2024-08-02 14:54:40 UTC
How does one test this?
Comment 3 David Cook 2024-10-24 06:39:53 UTC
Ohh I am curious about this as well
Comment 4 Martin Renvoize (ashimema) 2024-11-06 17:29:52 UTC
I found this whilst hitting a Z39.50 server that was misconfigured somewhere.. it's since working so I'm not able to offer a simple test plan.. 

The code is simple to read however.. we're just grabbing the message as well as the code now and displaying where we once only displayed the error code.

It makes debugging Z connections easier basically.
Comment 5 Magnus Enger 2025-01-07 12:29:52 UTC
Could it be possible to enable the publicserver in the Koha Zebra config, and have Koha talk to it's own Z39.50 server?
Comment 6 Magnus Enger 2025-01-07 12:58:16 UTC
Yes we can! 

Add a new Z39.50 server with these settings: 

Hostname: localhost
Port: 2100
Database: biblios
Syntax: MARC21/USMARC

But not sure how to tweak the config to make it return relevant error codes? I tried changing 

<retrieval syntax="usmarc" name="marc21"/>

to 

<retrieval syntax="usmarc" name="nope"/>

in /etc/koha/sites/kohadev/z3950/config.xml, but that still gives results. There was something else I tried that just gave "No results".
Comment 7 Magnus Enger 2025-01-07 12:59:25 UTC
PS

I tested this in KTD, and started it with "ktd --es7 up", since the "Z39.50 responder" relies on Elasticsearch.
Comment 8 Martin Renvoize (ashimema) 2025-01-07 14:07:24 UTC
This is such a trivial and clear bugfix in code. It really helps debug Z connections down the line if you come across any.
Comment 9 David Cook 2025-01-13 00:10:14 UTC
I also tried messing with the Zebra config but with no luck, but I was able to trigger the code added by the patch, so yay! 

Basically, I uncomment "publicserver" in the koha-conf.xml, restart the Zebra server, make note of the TCP port used by zebrasrv (you could assign a port in koha-conf.xml but anyway...), setup the Koha Z39.50 server, but set it to UNIMARC instead of USMARC. 

Then when I do a Keyword search for "test", I get the following:

KOHA record 1: 239 - Record syntax not supported
KOHA record 2: 239 - Record syntax not supported

-

I am going to mark it as Failed QA for now, because I think this is misleading.

Instead of "record", I think I'd say "result". I'm going to upload a screenshot of my test, and then add some more commentary...
Comment 10 David Cook 2025-01-13 00:10:35 UTC
Created attachment 176422 [details]
Example error message
Comment 11 David Cook 2025-01-13 00:12:37 UTC
(In reply to Magnus Enger from comment #7)
> PS
> 
> I tested this in KTD, and started it with "ktd --es7 up", since the "Z39.50
> responder" relies on Elasticsearch.

Note that my test plan doesn't work with the z3950_responder.pl since it appears to ignore the syntax sent with the Z39.50 query...
Comment 12 David Cook 2025-01-13 00:17:52 UTC
(In reply to David Cook from comment #9)
> I am going to mark it as Failed QA for now, because I think this is
> misleading.
> 
> Instead of "record", I think I'd say "result". I'm going to upload a
> screenshot of my test, and then add some more commentary...

I think it would be wise to wrap the Z39.50 error message in quotes. At first, I thought 239 might be the bib number, and then I thought there might be a bug... and then I realised that "239" is actually the error code. 

So it might also be worthwhile to write it out like...

"[Error code: 239] - Record syntax not support"

If you look at https://www.loc.gov/z3950/agency/contributions/1.html they suggest a "Recommended display" but it would require way too much work I reckon. But... we could do a hybrid like:

"Record syntax not supported (ref. 239)"

Because then I search "z39.50 ref 239" in a search engine, and I get useful data for my first hit.
Comment 13 David Cook 2025-01-13 00:20:08 UTC
Sorry for all the messages...

Additionally, it could be worthwhile wrapping the server data too: "[Server: KOHA]"

Alternatively, I suppose to be more in line with other errors that say "failed to SERVER" or "timeout to SERVER":

"Record syntax not supported (ref. 239) from KOHA"

--

Anyway, now we have a reproducible test plan, this should be an easy one to move forward :D
Comment 14 Martin Renvoize (ashimema) 2025-01-13 07:52:06 UTC
Created attachment 176423 [details] [review]
Bug 37546: Output error message from ZOOM in Z3950 errors
Comment 15 Martin Renvoize (ashimema) 2025-01-13 07:52:08 UTC
Created attachment 176424 [details] [review]
Bug 37546: Error message output improvements

As suggested by David, I've re-arranged the message output so we pass
'message' first, followed by the error reference inside brackets,
followed by "for SERVER" and finally "result No."

This follows the Z39.50 suggestions more closely whilst still including
our level of detail for Server + Result number.
Comment 16 Martin Renvoize (ashimema) 2025-01-13 07:52:22 UTC
Hows that ?
Comment 17 David Cook 2025-01-14 04:57:55 UTC
(In reply to Martin Renvoize (ashimema) from comment #16)
> Hows that ?

Good enough for me :)
Comment 18 David Cook 2025-01-14 04:58:51 UTC
Created attachment 176482 [details] [review]
Bug 37546: Output error message from ZOOM in Z3950 errors

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 19 David Cook 2025-01-14 04:58:54 UTC
Created attachment 176483 [details] [review]
Bug 37546: Error message output improvements

As suggested by David, I've re-arranged the message output so we pass
'message' first, followed by the error reference inside brackets,
followed by "for SERVER" and finally "result No."

This follows the Z39.50 suggestions more closely whilst still including
our level of detail for Server + Result number.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 20 David Cook 2025-01-14 05:01:33 UTC
(In reply to Martin Renvoize (ashimema) from comment #16)
> Hows that ?

Oh man... looking at it again today... your change was even smaller than I realised. Sorry for my earlier feedback. That misleading wording was already there. Blah.