feat(sipi): Improve error message if XSL file not found #1590

benjamingeer · 2020-02-05T12:22:29Z

This PR improves the error returned if Knora requests a nonexistent XSL transformation file from Sipi.

Resolves #1580.

benjamingeer · 2020-02-05T14:27:06Z

@flavens Could you check if this branch gives you a better error message?

flavens · 2020-02-05T16:05:49Z

@flavens Could you check if this branch gives you a better error message?

I get back this message:

I think it is better, however, I would have search for a long time it was the BEOL server file at the wrong place! I still need more experience with errors to get a better instinct :)

benjamingeer · 2020-02-05T16:20:08Z

@flavens Hmm, that's not what it's supposed to say. Can you give me a simple way to make this error happen?

benjamingeer · 2020-02-05T16:30:32Z

It's supposed to say something like "Unable to get XSL transformation file XYZ from Sipi"...

flavens · 2020-02-06T07:29:04Z

@flavens Hmm, that's not what it's supposed to say. Can you give me a simple way to make this error happen?

You can checkout the master branch of Knora-app.

Run on your terminal: yarn install --prod=false + ng s (you have to install yarn and angular-cli)

Then go to http://0.0.0.0:4200/system/users after log in (you can use the root user profile) and try to Add a user as system admin:

See error in the console.

benjamingeer · 2020-02-06T08:14:43Z

% yarn install --prod=false + ng s
yarn install v1.22.0
error `install` has been replaced with `add` to add new dependencies. Run "yarn add + ng s" instead.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

benjamingeer · 2020-02-06T08:15:24Z

% yarn add --prod=false + ng s
yarn add v1.22.0
[1/4] 🔍  Resolving packages...
error An unexpected error occurred: "https://registry.yarnpkg.com/+: Not found".
info If you think this is a bug, please open a bug report with the information provided in "/Users/benjamingeer/git/knora-app/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.

flavens · 2020-02-06T08:25:36Z

% yarn add --prod=false + ng s
yarn add v1.22.0
[1/4] 🔍  Resolving packages...
error An unexpected error occurred: "https://registry.yarnpkg.com/+: Not found".
info If you think this is a bug, please open a bug report with the information provided in "/Users/benjamingeer/git/knora-app/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.

you run yarn **install** not yarn add first (make sure yarn is installed) and then, ng s(with angular-cli installed on your computer)
No combination of the 2 commands

benjamingeer · 2020-02-06T08:58:43Z

you run yarn install not yarn add first

I did that and got an error message that told me to run yarn add instead, as shown in my comment above:

#1590 (comment)

The error message says "error install has been replaced with add to add new dependencies. Run "yarn add + ng s" instead."

But yarn add doesn't work either, it gives a different error message.

musicEnfanthen · 2020-02-06T10:04:38Z

Can confirm that the whole command yarn install --prod=false + ng s is not working. You would have to run it sequentially:

yarn install --prod=false

[wait for yarn to complete installation]

ng s [optional with ng s -o, then the app opens directly in the browser]

EDIT: It has to be install not add to install the whole list of dependencies. Add just installs a single package specified in the add command. So in the case of your second not working command (yarn add --prod=false + ng s), it would try to add the three packages +, ng and s which are obviously not existing.

benjamingeer · 2020-02-06T10:05:25Z

@musicEnfanthen Thanks, I just figured out the same thing at the same time. :)

benjamingeer · 2020-02-06T11:31:51Z

@flavens I did what you described, and I can see in the console that the request is empty ({}). That's why you're getting the error "The request content was malformed: No data sent in API request." Maybe that's because of a bug in knora-app.

In any case, I don't think that can have anything to do with the issue about an XSL file that doesn't exist. The request to create an admin user uses the admin API, which doesn't do anything with XSL templates. XSL templates are used to transform text markup in a resource.

flavens · 2020-02-06T12:56:03Z

@benjamingeer ah! an idea: I spotted this error before the release of knora-app. In the master branch, we have the newest knora-api-js-lib. To reproduce the error, you could try with knora-api-js-lib v0.1.1, knora-api v11.0.0 and place the BEOL server file in another folder.

benjamingeer · 2020-02-06T13:20:23Z

@flavens OK, I've actually just added an e2e test for this, which confirms that if you request a resource with an XSL transformation, and the XSL file doesn't exist, you get an HTTP 500 error with the message "Unable to get XSL transformation file".

I don't know how to check this manually using the BEOL data, but I guess you would have to request a BEOL resource. @tobiasschweizer can you help?

tobiasschweizer · 2020-02-06T14:01:49Z

webapi/src/main/scala/org/knora/webapi/store/iiif/SipiConnector.scala

            responseStr <- doSipiRequest(request)
        } yield SipiGetTextFileResponse(responseStr)
+
+        sipiResponseTry.recover {
+            case badRequestException: BadRequestException => throw SipiException(s"Unable to get XSL transformation file $xsltFileUrl from Sipi: ${badRequestException.message}")


sipiGetXsltTransformationRequestV2 is called in two cases: for XSLT used by StandoffResponderV2 to turn XML into HTML and for XSLT used the TEI transformer in ResourcesResponderV2. So I guess testing it with the TEI route is fine.

However, SipiGetTextFileRequest is as the message sent to SipiConnector that then internally calls sipiGetXsltTransformationRequestV2 seems strange to me. Shouldn't either the message be more specific or the method more generic?

In the future, Knora might require other text files from Sipi than XSL transformations and then the error message thrown by sipiGetXsltTransformationRequestV2 might be misleading.

Quite true, I'll fix that.

Done in 080dd44.

benjamingeer · 2020-02-07T11:54:00Z

@tobiasschweizer @flavens Is this better now?

tobiasschweizer

Ok, I think like this it is much better.

Is there a way to tell the client why Knora tried to get a text file from Sipi? There are two cases at the moment : one in the TEI route and one in the standoff route.

benjamingeer · 2020-02-07T12:57:17Z

Is there a way to tell the client why Knora tried to get a text file from Sipi?

What would you like it to say?

tobiasschweizer · 2020-02-07T13:06:09Z

What would you like it to say?

It would be great if the message would tell who sent the message to SipiResponder, e.g. the StandoffResponderV2.

benjamingeer · 2020-02-07T13:06:51Z

You'd like it to say, "Requested by StandoffResponderV2"?

tobiasschweizer · 2020-02-07T13:13:00Z

You'd like it to say, "Requested by StandoffResponderV2"?

Yes, but only if the message was sent by StandoffResponder. Can you find out about this in SipoResponder?

benjamingeer · 2020-02-07T13:13:36Z

Can you find out about this in SipoResponder?

I could add it to the message.

tobiasschweizer · 2020-02-07T13:14:15Z

I could add it to the message.

That would great. It is like a stack trace.

benjamingeer · 2020-02-07T14:25:02Z

OK, now the error message is "Unable to get file http://0.0.0.0:1024/0801/missing.xsl from Sipi as requested by org.knora.webapi.responders.v2.StandoffResponderV2: Sipi responded with HTTP status code 404: Not Found".

tobiasschweizer

That's great, thanks!

benjamingeer · 2020-02-07T15:54:25Z

@tobiasschweizer @flavens Thanks to both of you for your help with this.

feat(sipi): Improve error message if XSL file not found.

46c77d0

benjamingeer added the API/V2 label Feb 5, 2020

benjamingeer added this to the 2020-02 milestone Feb 5, 2020

benjamingeer self-assigned this Feb 5, 2020

benjamingeer mentioned this pull request Feb 5, 2020

Ben's PR history #571

Open

Benjamin Geer added 2 commits February 5, 2020 15:24

Merge branch 'develop' into wip/1580-xsl-not-found

cbc8e25

style(sipi): Optimise imports.

3f58d6f

fix(sipi): Preserve nested exception.

029291d

test(sipi): Add test for missing XSL transformation.

2c5ed5c

benjamingeer requested review from tobiasschweizer and flavens February 6, 2020 13:20

tobiasschweizer reviewed Feb 6, 2020

View reviewed changes

subotic modified the milestones: 2020-02, 2020-01 Feb 7, 2020

feat(sipi): Rename method, make error message less specific.

080dd44

tobiasschweizer reviewed Feb 7, 2020

View reviewed changes

feat(sipi): Include sender in text file request message.

0f34ace

tobiasschweizer approved these changes Feb 7, 2020

View reviewed changes

flavens approved these changes Feb 7, 2020

View reviewed changes

benjamingeer merged commit bbb42f6 into develop Feb 7, 2020

benjamingeer deleted the wip/1580-xsl-not-found branch February 7, 2020 15:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(sipi): Improve error message if XSL file not found #1590

feat(sipi): Improve error message if XSL file not found #1590

benjamingeer commented Feb 5, 2020 •

edited

Loading

benjamingeer commented Feb 5, 2020

flavens commented Feb 5, 2020

benjamingeer commented Feb 5, 2020

benjamingeer commented Feb 5, 2020

flavens commented Feb 6, 2020 •

edited

Loading

benjamingeer commented Feb 6, 2020

benjamingeer commented Feb 6, 2020

flavens commented Feb 6, 2020 •

edited

Loading

benjamingeer commented Feb 6, 2020 •

edited

Loading

musicEnfanthen commented Feb 6, 2020 •

edited

Loading

benjamingeer commented Feb 6, 2020

benjamingeer commented Feb 6, 2020

flavens commented Feb 6, 2020

benjamingeer commented Feb 6, 2020

tobiasschweizer Feb 6, 2020

tobiasschweizer Feb 6, 2020

benjamingeer Feb 6, 2020

benjamingeer Feb 7, 2020

benjamingeer commented Feb 7, 2020

tobiasschweizer left a comment

benjamingeer commented Feb 7, 2020

tobiasschweizer commented Feb 7, 2020 •

edited

Loading

benjamingeer commented Feb 7, 2020

tobiasschweizer commented Feb 7, 2020

benjamingeer commented Feb 7, 2020

tobiasschweizer commented Feb 7, 2020

benjamingeer commented Feb 7, 2020

tobiasschweizer left a comment

benjamingeer commented Feb 7, 2020

feat(sipi): Improve error message if XSL file not found #1590

feat(sipi): Improve error message if XSL file not found #1590

Conversation

benjamingeer commented Feb 5, 2020 • edited Loading

benjamingeer commented Feb 5, 2020

flavens commented Feb 5, 2020

benjamingeer commented Feb 5, 2020

benjamingeer commented Feb 5, 2020

flavens commented Feb 6, 2020 • edited Loading

benjamingeer commented Feb 6, 2020

benjamingeer commented Feb 6, 2020

flavens commented Feb 6, 2020 • edited Loading

benjamingeer commented Feb 6, 2020 • edited Loading

musicEnfanthen commented Feb 6, 2020 • edited Loading

benjamingeer commented Feb 6, 2020

benjamingeer commented Feb 6, 2020

flavens commented Feb 6, 2020

benjamingeer commented Feb 6, 2020

tobiasschweizer Feb 6, 2020

Choose a reason for hiding this comment

tobiasschweizer Feb 6, 2020

Choose a reason for hiding this comment

benjamingeer Feb 6, 2020

Choose a reason for hiding this comment

benjamingeer Feb 7, 2020

Choose a reason for hiding this comment

benjamingeer commented Feb 7, 2020

tobiasschweizer left a comment

Choose a reason for hiding this comment

benjamingeer commented Feb 7, 2020

tobiasschweizer commented Feb 7, 2020 • edited Loading

benjamingeer commented Feb 7, 2020

tobiasschweizer commented Feb 7, 2020

benjamingeer commented Feb 7, 2020

tobiasschweizer commented Feb 7, 2020

benjamingeer commented Feb 7, 2020

tobiasschweizer left a comment

Choose a reason for hiding this comment

benjamingeer commented Feb 7, 2020

benjamingeer commented Feb 5, 2020 •

edited

Loading

flavens commented Feb 6, 2020 •

edited

Loading

flavens commented Feb 6, 2020 •

edited

Loading

benjamingeer commented Feb 6, 2020 •

edited

Loading

musicEnfanthen commented Feb 6, 2020 •

edited

Loading

tobiasschweizer commented Feb 7, 2020 •

edited

Loading