Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Cannot differentiate installation issues by error code #4686

Open
line-o opened this issue Jan 14, 2023 · 11 comments
Open

Cannot differentiate installation issues by error code #4686

line-o opened this issue Jan 14, 2023 · 11 comments
Labels
enhancement new features, suggestions, etc. in progress Issue is actively being worked upon needs documentation Signals issues or PRs that will require an update to the documentation repo

Comments

@line-o
Copy link
Member

line-o commented Jan 14, 2023

Describe the bug

If an error is encountered running repo:install-and-deploy-from-db('/db/path/to/my.xar', 'https://my-repo/find')
will always throw an error with experr:EXPATH00.

When caught its $err:description will always be empty.

This code is defined in org.exist.xquery.modules.expathrepo.EXPathErrorCode as

public final static ErrorCode EXPDY007 = new EXPathErrorCode("EXPATH00", null);

On top of that the error codes do not follow the schema defined in the same class.

EXPATH specific errors

Stated: EXP(DY|SE|ST)[0-9]{4}
Actual: EXPATH[0-9]{2,3}

Sadly, the error codes do not seem to be defined by the EXPath pkg specification.
Does that actually mean we are free to use whatever codes we like?

Expected behavior

I would expect that specific error codes are used depending on the problem that was encountered.
These codes are defined in the class and also used in other repo-module implementation code.

public final static ErrorCode EXPDY001 = new EXPathErrorCode("EXPATH001", "Package not found.");
public final static ErrorCode EXPDY002 = new EXPathErrorCode("EXPATH002", "Bad collection URI.");
public final static ErrorCode EXPDY003 = new EXPathErrorCode("EXPATH003", "Permission denied.");
public final static ErrorCode EXPDY004 = new EXPathErrorCode("EXPATH004", "Error in descriptor found.");
public final static ErrorCode EXPDY005 = new EXPathErrorCode("EXPATH005", "Invalid repo URI");
public final static ErrorCode EXPDY006 = new EXPathErrorCode("EXPATH006", "Failed to connect to public repo");

To Reproduce

Just to highlight one of the many problems below is a XQSuite test that tries to install a package from a db-local XAR that is non-existent.

xquery version "3.1";

module namespace t="http://exist-db.org/xquery/test";

declare namespace test="http://exist-db.org/xquery/xqsuite";

declare
    %test:assertError("experr:EXPATH001")
function t:test() {
    repo:install-and-deploy-from-db('/db/path/to/my.xar', 'https://my-repo/find')
};

Context (please always complete the following information):
Build: eXist-6.1.0-SNAPSHOT (${build-commit})
Java: 1.8.0_352 (Azul Systems, Inc.)
OS: Mac OS X 12.6.2 (aarch64)

Additional context

  • How is eXist-db installed? built from source
  • Any custom changes in e.g. conf.xml? none
@line-o
Copy link
Member Author

line-o commented Jan 14, 2023

The repo module declares repo:install-from-db and repo:install-and-deploy-from-db and their implementation differs substantially.

@line-o line-o added the triage issue needs to be investigated label Jan 14, 2023
@line-o line-o changed the title [BUG] installation issues cannot be separated installation issues cannot be separated Jan 14, 2023
@line-o line-o changed the title installation issues cannot be separated Cannot differentiate installation issues by error code Jan 14, 2023
@line-o
Copy link
Member Author

line-o commented Jan 16, 2023

BaseX' implementation distinguishes between the errors (see https://docs.basex.org/wiki/Repository_Module#Errors)

  • not-found: a package does not exist.
  • descriptor: the package descriptor is invalid.
  • installed: the module contained in the package to be installed is already installed as part of another package.
  • parse: an error occurred while parsing the package.
  • version: the package version is not supported.

@line-o
Copy link
Member Author

line-o commented Jan 16, 2023

Since Version 9.0 BaseX bound all errors to the repo module.

Updated: error codes updated; errors now use the module namespace

I like this change as it makes it easier for XQuery modules calling repo functions to catch errors that might occur.

In eXist we currently have to declare a separate namespace experr in order to be able to catch them.

declare namespace repo="http://exist-db.org/xquery/repo";
declare namespace experr="http://expath.org/ns/error";

try {
  repo:undeploy('my/app')
}
catch experr:* {
  (: handle the error :)
}

@joewiz
Copy link
Member

joewiz commented Jan 16, 2023

The approach BaseX has taken makes a lot of sense.

@adamretter
Copy link
Contributor

Since Version 9.0 BaseX bound all errors to the repo module.

What is the namespace for that module in BaseX?

@joewiz
Copy link
Member

joewiz commented Jan 16, 2023

It's a BaseX-specific module:

All functions and errors in this module are assigned to the http://basex.org/modules/repo namespace, which is statically bound to the repo prefix.

See https://docs.basex.org/wiki/Repository_Module.

@adamretter
Copy link
Contributor

So I think there are two things separate things to think about here, one which I think is a good idea, and one which I think needs a little thought...

  1. I think as @line-o has suggested there needs to be a variety of error codes to help correctly identify a problem when it occurs. That's a good idea.

  2. If these errors are specific to the eXist-db implementation of the repo XQuery module, then they can go into the repo modules namespace. However, if they are described in any way in the EXPath spec, (or should perhaps be described in the EXPath spec), then they should remain in an EXPath namespace. I don't think the EXPath project has a spec for an XQuery repo extension module, but perhaps it should?

@line-o
Copy link
Member Author

line-o commented Jan 16, 2023

The XPath spec does not mention any error codes as I wrote.

@line-o
Copy link
Member Author

line-o commented Jan 16, 2023

I am pleased to read that we agree on the two main points. Change the implementation to throw specific errors and put them in the namespace of the repo module.

@line-o line-o added enhancement new features, suggestions, etc. in progress Issue is actively being worked upon needs documentation Signals issues or PRs that will require an update to the documentation repo and removed triage issue needs to be investigated labels Jan 17, 2023
@line-o
Copy link
Member Author

line-o commented Jan 17, 2023

The first step now is to introduce the new error codes in the repo module and throw them for specific error classes, even if we have to resort to parse the message of a thrown PackageException.

@line-o
Copy link
Member Author

line-o commented Jan 17, 2023

Next step:

  • implement the EXPath packaging logic ourselves
  • change the reference implementation to throw specific exception types

DrRataplan added a commit to eeditiones/roaster that referenced this issue Aug 21, 2024
Can occur when deploying apps. There is a value though!

this is caused by a bug on exist-db: eXist-db/exist#4686
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement new features, suggestions, etc. in progress Issue is actively being worked upon needs documentation Signals issues or PRs that will require an update to the documentation repo
Projects
None yet
Development

No branches or pull requests

3 participants