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

Failed when I try to add a proto file with import syntax. #13

Closed
DoneSpeak opened this issue Apr 21, 2020 · 12 comments
Closed

Failed when I try to add a proto file with import syntax. #13

DoneSpeak opened this issue Apr 21, 2020 · 12 comments
Assignees

Comments

@DoneSpeak
Copy link

DoneSpeak commented Apr 21, 2020

Protoman-0.3.1.dmg

image

Failed when I try to add the following protos.

account/api/account_test.proto -> Protobuf cannot be read

syntax = "proto3";

import "account/data/account.proto";

message AccountList {
    repeated account.data.Account accounts = 1;
}

account/data/account.proto -> Added successfully.

syntax = "proto3";

package account.data;

import "common/valid.proto";

message Account {
    int64 account_id = 1;
    string first_name = 2 [(valid.pattern) = "[a-zA-Z]{5,20}"];
    string last_name = 3 [(valid.pattern) = "[a-zA-Z]{5,20}"];
    string email = 4 [(valid.email) = true];
    string phone = 5 [(valid.pattern) = "[a-zA-Z]{5,20}"];
}
@spluxx spluxx self-assigned this Apr 21, 2020
@spluxx
Copy link
Owner

spluxx commented Apr 21, 2020

Thanks for the issue! I'll investigate and let you know. It seems like an issue with what root directory we use to parse the protobuf files.

spluxx pushed a commit that referenced this issue Apr 21, 2020
@spluxx spluxx mentioned this issue Apr 21, 2020
@spluxx spluxx closed this as completed Apr 21, 2020
@DoneSpeak
Copy link
Author

DoneSpeak commented Apr 21, 2020

@spluxx If I want to build a dmg package from source code, what do I need to do?

@spluxx
Copy link
Owner

spluxx commented Apr 21, 2020

Run npm run dist:mac :) Did the patch work for you?

@DoneSpeak
Copy link
Author

DoneSpeak commented Apr 22, 2020

Run npm run dist:mac :) Did the patch work for you?

Thanks, the patch works for me. But I found another problem, as follow:

image

I have imported all proto files into Protoman, and tried to send a message, but it failed.

no such type: .account.rest.api.CreateAccountRequest

As you can see, .account.rest.api.CreateAccountRequest has been imported and chosen as the request message.

@spluxx spluxx reopened this Apr 22, 2020
@spluxx
Copy link
Owner

spluxx commented Apr 23, 2020

@DoneSpeak would it be possible for you to try the fix on the v0.3.3 branch and see if there are additional problems? Thank you for the patience :)

You can run
npm run build (just ctrl+C after it's done building - you can ignore the bundle analyzer)
npm run start
to check it out without packaging it.

@DoneSpeak
Copy link
Author

No problem. I will take more try today. It's my honor to help you to improve Protoman. I holp it will work well.

@DoneSpeak
Copy link
Author

image

> Protoman@0.3.3 start /Users/yangguanrong/Codes/Github/Protoman
> electron dist/main.js

MAIN PROCESS STARTED
Initializing events
data directory: /Users/yangguanrong/Library/Application Support/Electron/data
Made sure datadir exists
Starting cleanup process
Finished initializing.
Loading most recent state...
Done loading most recent state...
Saving...
Saving...
Saving...
Making request with nonce: 4464740
Saving...

I don't know what's that means.

index out of range: 128 + 10 > 128

@spluxx
Copy link
Owner

spluxx commented Apr 23, 2020

index out of range: 128 + 10 > 128

This means that the given buffer had 10 more bytes than what's expected from the schema.

Perhaps the data that you're sending back from the server doesn't match what you selected on "Expected Message" tab?

The only way I was able to replicate that error was by setting the wrong expected message.

@DoneSpeak
Copy link
Author

DoneSpeak commented Apr 24, 2020

You are right. I tried again and it worked. Maybe you can add a panel to show the raw response which would be very helpful to let the user speculate the error reason. Of course, it would be better if the message shows the probable cause.

Do you consider to add a feature to add the whole folder as/to a collection?

@spluxx
Copy link
Owner

spluxx commented Apr 24, 2020

Nice! For the improved error messages and adding all protobuf files in folder-level feature, I'll create a separate issue for them and tag you on it :) Thank you for the contribution!

@spluxx
Copy link
Owner

spluxx commented Apr 24, 2020

Oh and @DoneSpeak if you want to try tackling one of those issues let me know :)

@spluxx spluxx mentioned this issue Apr 24, 2020
@DoneSpeak
Copy link
Author

DoneSpeak commented Apr 24, 2020

You are welcome. Yes, of course. I would love to do it. And I will spend some time to learn react + typescript + electron.

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

No branches or pull requests

2 participants