-
Notifications
You must be signed in to change notification settings - Fork 222
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
Better error reporting #594
Comments
I have tried to have a consistent "result":"success" for a properly submitted api request and an "error":"error message" if an error occurred. maybe a few slipped through the cracks... which commands return "status":"active" without a "result":"success"? I will fix that |
I'm not sure off the top of my head but I'll keep track of them as I notice them and report back here 👍 |
Ok, so I'm getting consistent errors but realised we're on a pretty old build of mm. Wanted to test on the latest version from I'm trying to build the latest version with my Docker image but builds are failing (exact same build script as before). Did something change with the build process? I can't see any changes documented. You can view the commands used and error log here: https://hub.docker.com/r/lukechilds/barterdex-api/builds/bcesucp8e6fs8vcy5pghfhn/ |
nothing changed in the build process |
How can you tell it's nanomsg? Are you inferring that from the
errors? Are you certain nothing changed in the build process on your end? The exact same build script worked previously: https://hub.docker.com/r/lukechilds/barterdex-api/builds/bfdhvzqpzsi9baq8ueo9psh/ I haven't made any changes to the Dockerfile. And looking through the commit history here it looks like there have been some nanomsg related changes recently: #582 #583 Could this be related? Should we be building with |
probably good idea |
@satindergrewal I can build with my Docker image from this commit: a484a94 Looks like you made quite a few changes to the build system after that and my Docker image can no longer create builds. What build process are you using now? Have you tested it on both macOS and Linux? |
@lukechilds I did not made change to In this process I added a new script ( These are the PR and commits you will find helpful in which I pushed changes: Hope these will help resolve your issues. |
@satindergrewal Yeah you can see those changes appear to have broken Here is a successful build before the changes: And here is an identical build after the changes which fails: Are there any extra undocumented steps I should be taking after your changes or is everything supposed to work the same as before?
Did you mean to say that the other way round? As far as I can see in this PR (#585) |
oh yes, that's correct linux dymamic and mac static mm :) the failed docker log link says this in log
and line 4 in m_mm is wonder if for some reason docker is complaining about this if statement. Can you check in normal non-docker environment in a linux bash shell if you get same error or it works for you as normal? I don't know much about docker since I never had to try or needed. Please try things first in linux environment, and report me if that fails. I think that would help troubleshoot this issue better. |
Sure, will test that now. Also, it would probably be better to just assume the normal behaviour by default and apply macOS specific overrides inside an if statement. For example the current |
Found a couple of bugs in the build script, sent over a patch. @jl777 @satindergrewal are you able to review this: #597 |
… (jl777#594) * Call Github release API via curl to check that assets are uploaded. * Call Github release API via curl to check that assets are uploaded. * Call Github release API via curl to check that assets are uploaded. * Call Github release API via curl to check that assets are uploaded. * Call Github release API via curl to check that assets are uploaded. * Call Github release API via curl to check that assets are uploaded. * Call Github release API via curl to check that assets are uploaded. * Log DEBUG_UPLOADED and RELEASE_UPLOADED. * Use COMMIT_TAG from env to determine DEBUG_UPLOADED and RELEASE_UPLOADED. * Use TAG from env to determine DEBUG_UPLOADED and RELEASE_UPLOADED. * Build and publish to Github only when corresponding binaries are not uploaded. * Fix bash if on Setup ENV. * Check the git tag instead of hard code. * Clean upload dir -> Recreate upload dir. * If commit is tagged edit existing release, create new release if there's no tag. * Always echo DEBUG_UPLOADED, RELEASE_UPLOADED and RELEASE_TAG. * Try to fix RELEASE_TAG when commit is not tagged. * echo $(Build.BuildId) * Echo if commit tag is empty. * Use [ -z $TAG ] to determine if $TAG is empty. * Enable the Build and test Debug stage back. * Release only on mm2 branch again.
During testing we're getting quite a few errors and it's very hard to tell what's causing them.
marketmaker
will occasionally hang, drop connections or just quit. Even when we query it one by one. It's very hard for us to debug and work out what's going wrong because there's often little or no error data in the responses (or no response at all).Also, of the responses that do report errors, many of them are inconsistent in how they indicate success/failure. For example, these are some of the variations we've came across so far:
Would it be possible to implement consistent error reporting across all of the commands?
The text was updated successfully, but these errors were encountered: