Project

General

Profile

Bug #3736

Opensearch returnerer i nogle tilfælde collections som booleans

Added by Simon Holt about 1 year ago. Updated 5 months ago.

Status:
Needs code review
Priority:
High
Target version:
Estimated time:
URL med eksempel:
Kategorier:
Integration - Brønd - Søg, rankér filtrér sortér

Description

Observeret i denne sag, hvor det førte til en PHP fatal error: https://platform.dandigbib.org/issues/3706 

PR i ovenstående sag fikser fatal error, men det tyder på et underliggende problem.

getNumTotalCollections returnerer samlet antal collection fra søgeresulatet, men nogle af dem kan potentielt være boolean når man får dem returneret via getTingEntityCollections() med opensearch som provider.


Related issues

Related to DDB CMS - Bug #3706: ding_message fejler og ødelægger cron (HVORDAN TESTES DENNE?)Resolved (tag version)
Related to DDB CMS - Bug #3827: Error: __clone method called on non-object i _opensearch_predict_cache() Resolved (tag version)

History

#1 Updated by Simon Holt about 1 year ago

  • Related to Bug #3706: ding_message fejler og ødelægger cron (HVORDAN TESTES DENNE?) added

#2 Updated by Rolf Madsen about 1 year ago

  • Status changed from New to Ready for development
  • Priority changed from Normal to High
  • Target version set to Release 30 - Søgegrænseflade

#3 Updated by Kasper Garnæs about 1 year ago

I forbindelse med løsningen af denne bør rettelsen i https://platform.dandigbib.org/issues/3827 genbesøges og muligvis fjernes hvis den ikke længere er relevant.

#4 Updated by Rolf Madsen 12 months ago

  • Related to Bug #3827: Error: __clone method called on non-object i _opensearch_predict_cache()  added

#5 Updated by Rolf Madsen 10 months ago

Hvad er konsekvensen ved dette issue?

#6 Updated by Christel Krabbenhøft 9 months ago

Hej Simon. Vil du svare denne sag?

#7 Updated by Simon Holt 8 months ago

MIg bekendt har vi ikke observeret nogle fejl forårsaget af denne udover den i #3706

Uden at gøre det alt for teknisk er problemet, at opensearch modulet skal (i følge det interface den implementerer) returnere en liste med værker fra søgeresultatet. Den skal ikke returnere en liste, hvor noget kan være FALSE, som fra eksemplet i #3706. Hvis noget kode (med rette) forventer at få værker kan det give fejl som i #3706.

Så jeg vil sige det bør rettes/undersøges, men har et par andre sager som er vigtigere lige nu, så får ikke kigget på det lige med det samme.

#8 Updated by Christel Krabbenhøft 8 months ago

  • Target version changed from Release 30 - Søgegrænseflade to Release 31 - bugfixes

#9 Updated by Christel Krabbenhøft 7 months ago

  • Target version changed from Release 31 - bugfixes to Release 33 - Bugfixes

#10 Updated by Simon Holt 7 months ago

  • Status changed from Ready for development to Needs code review
  • Assignee changed from Simon Holt to Gitte Barlach

Jeg har nu undersøgt denne nærmere og som jeg ser det, er der tale om to forskellige (men måske beslægtede) problemer.

Problem 1 fra #3706:

Det konkrete problem var her, at der blev kaldt en metode på det der burde være et værk-objekt, men som egentlig var en boolean. Som det fremgår af diskussion i sagen var det uklart, hvad problemet præcist var. Enten var værket en boolean returneret fra opensearch_do_search() eller også var der slet ingen collection og PHP's current metode returnerede FALSE. Der er ikke specificeret hvad det var for nogle søgninger, så det er ret svært at sige noget om.

Problem 2 fra #3827

Her er problemet at nogle materialer i et værk ikke er objekter, som de burde være. Det er ret svært at gennemskue, hvordan det lige kan ske. Jesper har i løsningen i #3827 indført logging af disse situationer.

Foreløbig "løsningsforslag":

Jeg foreslår at vi indtil videre sørger for at disse situationer logges med relevant data, så vi har et bedre grundlag for at kunne debugge. Det er vist det bedste vi kan gøre lige nu. Jeg har prøvet at gennemgå koden og se om jeg kunne gennemskue hvordan det kan ske og det ser faktisk ud til, at de her situationer ikke burde kunne opstå. Så det tyder på der sker en eller anden fejl eller også er det et eller andet kompliceret caching-problem. Under alle omstændigheder har vi brug for mere data omkring situationerne hvor de opstår.

Så her et PR, der tilføjer logging i watchdog for ovenstående situationer. Som sagt har Jesper allerede tilføjet logging for problem 2, så har bare tilføjet en kommentar om at det er midlertidig debug kode og et link til sagen her:

PR: https://github.com/ding2/ding2/pull/1368 

Lige at par spørgsmål til Christan/DBC: 

1. Kan det lade sig gøre at finde tilbage til de søgninger der forårsagede fejlen i problem 1 fra #3706?

2. Som sagt havde Jepser indført watchdog log af situationen beskrevet i problem 2. Vil det være muligt for DBC at kigge loggen igennem i drift for at se om der er fanget noget? De er på formen:

- 'Predict cache found collection with empty objects. Search query: @query'

Hvor @query er den søgestreng der forårsagede situationen.

De nye log-beskeder fra mit PR er på formen:

- 'The reported number of collections (@reported) does not match the actual number (@actual) in opensearch_do_search. Search query: @query'

- 'Found non-object collections on search result in opensearch_do_search. Search query: @query'

Men de vil selvfølgelig først begynde at dukke op, når/hvis PR bliver merget.

 

#11 Updated by Jesper Kristensen 7 months ago

Hej

Simon har du set denne som vi har lavet baseret på eReolen og cache: https://github.com/ding2/ding2/pull/1364

#12 Updated by Simon Holt 7 months ago

Ja, har taget et kort kig på den.

Men kan ikke rigtig se ud fra sagen hvilket problem der løses. Tror du dine rettelser der vil løse alle problemer her?

#13 Updated by Gitte Barlach 7 months ago

Til orientering er pull/1364, som Jesper omtaler i kommentar 11, fremsat i denne her sag: #4139

#14 Updated by Jesper Kristensen 7 months ago

@simon jeg ved det ikke... men synes jeg vil pege dig mod den... da det er samme kode område og cache vi slås med.

#15 Updated by Simon Holt 7 months ago

Ok. Er næsten sikker på problemerne er opstået pga et eller andet i cachen. Men det er stadig lidt gætteri baseret på at det ikke ser ud til at være muligt at komme i disse situaioner uden om cachen.

Måske ville det være godt at få merget PR, så vi kan få noget data og ikke mindst et billede af problemets omfang (hvis det da overhovedet stadig optræder, med alle de rettelser der er blevet i mellemtiden).

#16 Updated by Gitte Barlach 5 months ago

  • Assignee changed from Gitte Barlach to Jesper Kristensen

Also available in: Atom PDF