Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Internal Server Error on connect after upgrade from 0.1.2 to 0.2.0-dev2 when agent contains group hab #257

Closed
lenkan opened this issue Jun 26, 2024 · 14 comments · Fixed by #284

Comments

@lenkan
Copy link
Collaborator

lenkan commented Jun 26, 2024

I am having issues upgrading a keria instance from 0.1.2 to 0.2.0-dev2. When the instance contains an agent with a group hab, the controller cannot connect to it.

See reproduction here: https://github.com/lenkan/keria-upgrade-test. It should be possible to run it by cloning the repo and

git clone [email protected]:lenkan/keria-upgrade-test.git
cd keria-upgrade-test
./run-test.sh

The error output is:

connect-1  | /app/scripts-connect/node_modules/signify-ts/src/keri/app/controller.ts:152
connect-1  |         if (state == null || state['ee']['s'] == 0) {
connect-1  |                                        ^
connect-1  |
connect-1  |
connect-1  | TypeError: Cannot read properties of undefined (reading 's')
connect-1  |     at Controller (/app/scripts-connect/node_modules/signify-ts/src/keri/app/controller.ts:152:40)
connect-1  |     at SignifyClient.connect (/app/scripts-connect/node_modules/signify-ts/src/keri/app/clienting.ts:133:27)
connect-1  |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
connect-1  |     at <anonymous> (/app/scripts-connect/src/main.ts:13:1)
connect-1  |
connect-1  | Node.js v20.12.2
connect-1  | npm ERR! Lifecycle script `start` failed with error:
connect-1  | npm ERR! Error: command failed
connect-1  | npm ERR!   in workspace: undefined
connect-1  | npm ERR!   at location: /app/scripts-connect
connect-1 exited with code 1
keria-1  | /keripy/src/keri/core/serdering.py:120: SyntaxWarning: invalid escape sequence '\A'
keria-1  |   """Design Notes:
keria-1  | The Agency is loaded and waiting for requests...
keria-1  | 2024-06-26 13:21:20 [FALCON] [ERROR] GET /agent/EEFCS78eJySGGx9Nl7r-eHRSDZ4xbisWwkqrwEWft-bG => Traceback (most recent call last):
keria-1  |   File "falcon/app.py", line 365, in falcon.app.App.__call__
keria-1  |   File "/usr/local/var/keria/src/keria/app/aiding.py", line 110, in on_get
keria-1  |     agent = self.agency.get(caid)
keria-1  |             ^^^^^^^^^^^^^^^^^^^^^
keria-1  |   File "/usr/local/var/keria/src/keria/app/agenting.py", line 246, in get
keria-1  |     agentHby = habbing.Habery(name=caid, base=self.base, bran=self.bran, ks=ks, temp=self.temp)
keria-1  |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
keria-1  |   File "/keripy/src/keri/app/habbing.py", line 241, in __init__
keria-1  |     self.setup(**self._inits)  # finish setup later
keria-1  |     ^^^^^^^^^^^^^^^^^^^^^^^^^
keria-1  |   File "/keripy/src/keri/app/habbing.py", line 316, in setup
keria-1  |     self.loadHabs()
keria-1  |   File "/keripy/src/keri/app/habbing.py", line 345, in loadHabs
keria-1  |     hab = SignifyGroupHab(ks=self.ks, db=self.db, cf=self.cf, mgr=self.mgr,
keria-1  |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
keria-1  | TypeError: SignifyGroupHab.__init__() missing 1 required positional argument: 'smids'
keria-1  |

Could it be that the migration is not done correctly? If so, does anyone know how to correctly upgrade all the databases in a keria instance? Right now it does kli migrate run --name <agent aid> for all directories inside /usr/local/var/keri/db. See https://github.com/lenkan/keria-upgrade-test/blob/main/migrate.sh

Note that the upgrade and connection works if the agent does not contain a group.

@2byrds
Copy link
Collaborator

2byrds commented Jun 26, 2024

Thank you for reporting and taking the time to create a test to reproduce it @lenkan . I'm working #235 while on vacation. If this hasn't been addressed by the time I return I'll dive in to help.

@lenkan
Copy link
Collaborator Author

lenkan commented Jun 27, 2024

I had a look at the implementation, and I think this is not strictly related to upgrading, but rather just loading an agent after a reboot. Perhaps this is reproducible using 0.2.0-dev2 and simply:

  • Start the keria instance
  • Create signify controller and boot agent
  • Connect to agent
  • Create AID and group AID
  • Reboot keria instance
  • Connect to agent

@kentbull
Copy link
Contributor

I will take a look at this.

@m00sey
Copy link
Member

m00sey commented Jun 28, 2024

TypeError: SignifyGroupHab.__init__() missing 1 required positional argument: 'smids'

@kentbull
Copy link
Contributor

#261 should fix this.

@kentbull
Copy link
Contributor

@lenkan will you test this again now that #261 was merged?

@m00sey
Copy link
Member

m00sey commented Jun 28, 2024

Fixed by WebOfTrust/keripy#809

@lenkan
Copy link
Collaborator Author

lenkan commented Jul 1, 2024

Thanks a lot for swift fixing everyone!

I am still getting the same error with keria:0.2.0-dev3.

@m00sey Did you intend to include this fix in 0.2.0-dev3? It looks like it is still using keri:1.2.0-dev6, not dev8.

lenkan:~/code/keria-upgrade-test * main
$ docker run --entrypoint kli --rm  weboftrust/keria:0.2.0-dev3 version
/keripy/src/keri/core/serdering.py:120: SyntaxWarning: invalid escape sequence '\A'
  """Design Notes:
Library version: 1.2.0-dev6

lenkan:~/code/keria-upgrade-test * main
$ docker run --entrypoint cat --rm  weboftrust/keria:0.2.0-dev3 setup.py
#!/usr/bin/env python
# -*- encoding: utf-8 -*-
"""
$ python setup.py register sdist upload

First Time register project on pypi
https://pypi.org/manage/projects/


More secure to use twine to upload
$ pip3 install twine
$ python3 setup.py sdist
$ twine upload dist/keria-0.0.1.tar.gz


Update sphinx /docs
$ cd /docs
$ sphinx-build -b html source build/html
or
$ sphinx-apidoc -f -o source/ ../src/
$ make html

Best practices for setup.py and requirements.txt
https://caremad.io/posts/2013/07/setup-vs-requirement/
"""


from glob import glob
from os.path import basename
from os.path import splitext

from setuptools import find_packages
from setuptools import setup

setup(
    name='keria',
    version='0.2.0-dev3',  # also change in src/keria/__init__.py
    license='Apache Software License 2.0',
    description='KERIA: KERI Agent in the cloud',
    long_description="KERIA: KERI Agent in the cloud.",
    author='Philip S. Feairheller',
    author_email='[email protected]',
    url='https://github.com/WebOfTrust/keria',
    packages=find_packages('src'),
    package_dir={'': 'src'},
    py_modules=[splitext(basename(path))[0] for path in glob('src/*.py')],
    include_package_data=True,
    zip_safe=False,
    classifiers=[
        # complete classifier list: http://pypi.python.org/pypi?%3Aaction=list_classifiers
        'Development Status :: 3 - Alpha',
        'Intended Audience :: Developers',
        'License :: OSI Approved :: Apache Software License',
        'Operating System :: Unix',
        'Operating System :: POSIX',
        'Operating System :: Microsoft :: Windows',
        'Programming Language :: Python :: 3.10',
        'Programming Language :: Python :: Implementation :: CPython',
        # uncomment if you test on these interpreters:
        # 'Programming Language :: Python :: Implementation :: PyPy',
        # 'Programming Language :: Python :: Implementation :: IronPython',
        # 'Programming Language :: Python :: Implementation :: Jython',
        # 'Programming Language :: Python :: Implementation :: Stackless',
        'Topic :: Utilities',
    ],
    project_urls={
        'Issue Tracker': 'https://github.com/WebOfTrust/keria/issues',
    },
    keywords=[
        "secure attribution",
        "authentic data",
        "discovery",
        "resolver",
        # eg: 'keyword1', 'keyword2', 'keyword3',
    ],
    python_requires='>=3.12.2',
    install_requires=[
        'hio>=0.6.12',
        'keri>=1.2.0.dev6',
        'mnemonic>=0.20',
        'multicommand>=1.0.0',
        'falcon>=3.1.3',
        'http_sfv>=0.9.8',
        'dataclasses_json>=0.5.7',
        'apispec>=6.3.0',
    ],
    extras_require={
        # eg:
        #   'rst': ['docutils>=0.11'],
        #   ':python_version=="2.6"': ['argparse'],
    },
    tests_require=[
        'coverage>=5.5',
        'pytest>=6.2.4',
    ],
    setup_requires=[
    ],
    entry_points={
        'console_scripts': [
            'keria = keria.app.cli.keria:main',
        ]
    },
)

I can also see that 0.2.0-dev2 was published again in docker hub at the same time if that has anything to do with it?

@lenkan
Copy link
Collaborator Author

lenkan commented Jul 1, 2024

I also see that the docker image is still building from keri:1.2.0-dev6

FROM weboftrust/keri:1.2.0-dev6

That does not feel right to me as the keri version can differ between setup.py and the docker image. I can create a separate issue for that (edit: #263). But probably we want to use an python base image and install the dependencies using pip?

@lenkan
Copy link
Collaborator Author

lenkan commented Jul 1, 2024

Update: I have gotten past the first step by building keria locally. So I added some more steps to the upgrade test script. It now fails when calling the identifiers().members() endpoint.

connect-1  | /app/node_modules/signify-ts/src/keri/app/clienting.ts:214
connect-1  |             throw new Error(message);
connect-1  |                   ^
connect-1  |
connect-1  |
connect-1  | Error: HTTP GET /identifiers/group0/members - 500 Internal Server Error - {"title": "500 Internal Server Error"}
connect-1  |     at SignifyClient.fetch (/app/node_modules/signify-ts/src/keri/app/clienting.ts:214:19)
connect-1  |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
connect-1  |     at Identifier.members (/app/node_modules/signify-ts/src/keri/app/aiding.ts:470:21)
connect-1  |     at <anonymous> (/app/src/main.ts:22:31)
connect-1  |
connect-1  | Node.js v20.12.2
connect-1 exited with code 1
keria-1  | The Agency is loaded and waiting for requests...
keria-1  |   Agent: ED4AQf_aVbauZvO79AGHucq5gArqbYmmbQ-lEu9mMjNz   Controller: EEFCS78eJySGGx9Nl7r-eHRSDZ4xbisWwkqrwEWft-bG
keria-1  | 2024-07-01 09:35:18 [FALCON] [ERROR] GET /identifiers/group0/members => Traceback (most recent call last):
keria-1  |   File "falcon/app.py", line 365, in falcon.app.App.__call__
keria-1  |   File "/usr/local/var/keria/src/keria/app/aiding.py", line 2109, in on_get
keria-1  |     for smid in smids:
keria-1  | TypeError: 'NoneType' object is not iterable
keria-1  |

I am not quite sure what it is about.

Update is here https://github.com/lenkan/keria-upgrade-test

@m00sey
Copy link
Member

m00sey commented Jul 1, 2024

Sounds like smids is None

@m00sey
Copy link
Member

m00sey commented Jul 1, 2024

I also see that the docker image is still building from keri:1.2.0-dev6

FROM weboftrust/keri:1.2.0-dev6

That does not feel right to me as the keri version can differ between setup.py and the docker image. I can create a separate issue for that (edit: #263). But probably we want to use an python base image and install the dependencies using pip?

I pushed an updated keria image: https://hub.docker.com/repository/docker/weboftrust/keria/general

I have no opinion on #263

@iFergal
Copy link
Collaborator

iFergal commented Jul 17, 2024

@lenkan I think I've found the issue.

WebOfTrust/keripy#734 updated keripy to use smids and rmids everywhere instead of searching based on public keys (WebOfTrust/keripy#694), and added the migration script.

GroupHabs previously already stored smids and rmids anyway, so the migration script works for groups created with the kli.

However SignifyGroupHab didn't store this previously - so any groups created before this PR aren't migrated properly and losing the smids field.

--

This affects one of my dev instances, but it's not production so can be wiped on upgrade.

If you or anyone has something in production that needs to be migrated, maybe the migration script could be adjusted to get smids the old way (it's not part of a full release yet, only dev releases). But I also see this "old way" had the issue of #189.

@kentbull
Copy link
Contributor

Nice find, this sounds right. I'll take a look at this and see what a migration script including the new smids could be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants