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

Fix 160 #163

Merged
merged 1 commit into from
Sep 17, 2020
Merged

Fix 160 #163

merged 1 commit into from
Sep 17, 2020

Conversation

sheagcraig
Copy link
Collaborator

I took a look.

Sorry for the delay friend ;)

So, try firing up python3 and doing

from Foundation import NSString
NSString("All good 🌮")
NSString("Uh oh 💩".encode())
NSString(b"Definitely not")

So I removed the part where the send notification func is encoding messages prior to being stuffed into a notification and voila! Recipe Robot is happy again.

NSString doesn't know what to do with encoded bytes.
@sheagcraig
Copy link
Collaborator Author

Apparently I totally jacked the PR... I know you've got A+ git skills that eclipse mine, so please cherry pick or whatever magic incantation it requires ;)

@homebysix homebysix changed the base branch from master to 2.0.0-dev September 17, 2020 16:27
@homebysix
Copy link
Owner

Holy moly — all that searching down rabbit holes, and it was down to a two-line fix. You're a legend! 🏆

@homebysix homebysix merged commit b4a2f3b into 2.0.0-dev Sep 17, 2020
@homebysix homebysix deleted the fix_160 branch September 17, 2020 17:27
@sheagcraig
Copy link
Collaborator Author

After thinking about this for awhile, I was wondering... Maybe you should capture exceptions and throw the tracebacks into this NSNotification system for the app to show. I think that's what made this so difficult to troubleshoot for you. When it failed in the app, it never displayed the actual errors you were getting. But if you could have a top-level exception handler that pops the traceback into the NSNotification thingy, you'd be able to at least see it in the app log.

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 this pull request may close these issues.

2 participants