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

Output a clearer error message when publishing to an unregistered app #569

Closed
QuinnWilton opened this issue Jan 18, 2022 · 2 comments · Fixed by #570
Closed

Output a clearer error message when publishing to an unregistered app #569

QuinnWilton opened this issue Jan 18, 2022 · 2 comments · Fixed by #570
Labels
💗 enhancement New feature or request 🔰 good first issue Good for newcomers 🐇 quick Low effort to implement / fix — a few hours at most

Comments

@QuinnWilton
Copy link
Contributor

QuinnWilton commented Jan 18, 2022

Summary

When attempting to publish to an app that hasn't been registered, the error message should reflect this fact.

Problem

While setting up a new project, I created fission.yaml by hand, and manually entered my desired url. When I published the app, I received the following output:

$ fission app publish
🕛🛫 App publish local preflight
🕑✈️  Pushing to remotePeer List...
🚫 Server unable to sync data

I thought that perhaps the app wasn't being registered automatically, so I tried to register it, and the output suggested that the registration did in fact exist.

$ fission app register --name some-name
App already set up at some-name.fission.app

Publishing again, in verbose mode, made it clear that the app was unregistered:

FailureResponse (Request {requestPath = (BaseUrl {baseUrlScheme = Https, baseUrlHost = "runfission.com", baseUrlPort = 443, baseUrlPath = ""},"..."), requestQueryString = fromList [("copy-data",Just "true")], requestBody = Nothing, requestAccept = fromList [application/json;charset=utf-8,application/json], requestHeaders = fromList [("Authorization","<REDACTED>")]), requestHttpVersion = HTTP/1.1, requestMethod = "PUT"} (Response {responseStatusCode = Status {statusCode = 404, statusMessage = "Not Found"}, responseHeaders = fromList [("Date","Tue, 18 Jan 2022 01:28:56 GMT"),("Transfer-Encoding","chunked"),("Connection","keep-alive"),("Server","Warp/3.3.17")], responseHttpVersion = HTTP/1.1, responseBody = "App not found"})

Impact

The current error message suggested an issue with my IPFS configuration, so I deleted everything and tried again from scratch. My next guess was an availability issue with the remote peers, which might drive a new user to table Fission and assume it isn't ready for use.

Right now, there aren't many reasons to create a fission.yaml outside of the regular fission app register flow, but I can imagine this failure case becoming more common as the configuration options available grow in number and complexity. For example, someone may wish to copy their existing fission.yaml from another project, in order to copy environment config as described in #567. Similarly, you may begin to see templating tools that scaffold out Webnative applications, and this is a caveat they'd need to be aware of.

Solution

Since the response from the server is already a 404, this just needs to be translated to a human readable error. Perhaps:

🚫 The app has not yet been registered. Delete your fission.yaml and run: fission app register --name some-name

Requiring that the user delete their fission.yaml isn't a great experience, so it may also be worth supporting a --force flag for fission app register, that registers an app with the name part of the URL in fission.yaml. Then the error message could be adjusted to:

🚫 The app has not yet been registered. Register it by running: fission app register --force

@QuinnWilton QuinnWilton added the 💗 enhancement New feature or request label Jan 18, 2022
@github-actions
Copy link

Thank you for submitting an issue! It means a lot that you took the time -- it helps us be better 🙏

@expede
Copy link
Member

expede commented Jan 18, 2022

@QuinnWilton thanks for filing! Indeed I can reproduce this.

@expede expede added 🐇 quick Low effort to implement / fix — a few hours at most 🔰 good first issue Good for newcomers labels Jan 18, 2022
@expede expede closed this as completed Jan 26, 2022
@expede expede reopened this Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💗 enhancement New feature or request 🔰 good first issue Good for newcomers 🐇 quick Low effort to implement / fix — a few hours at most
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants